ci: use commit hash for github action, add persist-credentials false - #643
Closed
richm wants to merge 2 commits into
Closed
ci: use commit hash for github action, add persist-credentials false#643richm wants to merge 2 commits into
richm wants to merge 2 commits into
Conversation
The latest security guidance is to use the full commit hash, which is immutable, instead of a tag or version, which can be mutable, for the reference to a version of a github action. There are known attacks which inserted unauthorized code in a version tag and moved the tag. This prevents this sort of attack, at the cost of more maintenance burden, but dependabot will largely take care of this for us. Each version or tag has been replaced with the corresponding commit hash - in some cases, this is not the latest commit on the main branch, so I would expect to see some dependabot updates in the near future. I thought it was safer to do it this way - preserve existing behavior/functionality - rather than replace and upgrade to a newer version at the same time. The coderabbit `Pin the Galaxy action's container image.` issue is tracked at robertdebock/galaxy-action#16 This also adds `persist-credentials: false` to the actions/checkout tasks so that any credentials used by that task will not persist for subsequent tasks, for those workflows that do not need the credentials for subsequent tasks. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGitHub Actions workflows now pin external actions and selected Git dependencies to immutable commits. Checkout steps explicitly set credential persistence according to whether later workflow steps need repository write access. ChangesWorkflow dependency pinning
Possibly related issues
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![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
🧹 Nitpick comments (2)
.github/workflows/qemu-kvm-integration-tests.yml (1)
133-133: 🩺 Stability & Availability | 🔵 TrivialUse a supported Podman repository before Questing is archived.
The pinned action defaults
ubuntu-repositorytoquesting. Ubuntu 25.10 is EOL and will move toold-releases.ubuntu.com. Configure a supported runner and repository combination that provides Podman 5 or newer.🤖 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/qemu-kvm-integration-tests.yml at line 133, Update the redhat-actions/podman-install action configuration in the QEMU KVM integration workflow to use a supported Ubuntu runner and repository combination, explicitly selecting a repository that is not Questing and provides Podman 5 or newer. Preserve the existing pinned action reference and ensure the configured runner matches the repository..github/workflows/changelog_to_tag.yml (1)
82-82: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPrefer the runner-provided
ghcommand for release creation.The supplied static analysis identifies
ncipollo/release-actionas superfluous. GitHub-hosted runners provide the GitHub CLI, andgh release createsupports the current tag, title, notes-file, latest-release, and verify-tag behavior. Replacing this action removes one privileged third-party execution point. (cli.github.com)Possible replacement
- name: Create Release - id: create_release - uses: ncipollo/release-action@339a81892b84b4eeb0f6e744e4574d79d0d9b8dd # v1 - with: - tag: ${{ steps.tag.outputs.tagname }} - name: Version ${{ steps.tag.outputs.tagname }} - bodyFile: ./.tagmsg.txt - makeLatest: true + env: + GH_TOKEN: ${{ github.token }} + run: | + gh release create "${{ steps.tag.outputs.tagname }}" \ + --verify-tag \ + --title "Version ${{ steps.tag.outputs.tagname }}" \ + --notes-file ./.tagmsg.txt \ + --latestVerify the runner image exposes the required
gh release createoptions before applying this refactor.
This suggestion is based on the provided zizmor hint and GitHub CLI documentation.🤖 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/changelog_to_tag.yml at line 82, Replace the ncipollo/release-action step in the changelog workflow with the runner-provided gh release create command, preserving the existing tag, title, notes-file, latest-release, and verify-tag behavior. Confirm the configured runner supports these gh release create options and retain the workflow’s current release inputs and sequencing.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/build_docs.yml:
- Around line 28-30: Update the checkout step in the build-docs workflow to set
persist-credentials to true, ensuring the initial git push for a missing docs
branch can use the checkout-provided authentication.
- Around line 52-60: Update both later checkout steps in the workflow, including
the docs checkout and the README/template checkout, to disable persisted
credentials while preserving their existing refs and push-related settings.
Ensure the separate first-run branch-creation push is explicitly authenticated
or relies on docs being pre-created, without restoring persisted credentials on
these checkouts.
---
Nitpick comments:
In @.github/workflows/changelog_to_tag.yml:
- Line 82: Replace the ncipollo/release-action step in the changelog workflow
with the runner-provided gh release create command, preserving the existing tag,
title, notes-file, latest-release, and verify-tag behavior. Confirm the
configured runner supports these gh release create options and retain the
workflow’s current release inputs and sequencing.
In @.github/workflows/qemu-kvm-integration-tests.yml:
- Line 133: Update the redhat-actions/podman-install action configuration in the
QEMU KVM integration workflow to use a supported Ubuntu runner and repository
combination, explicitly selecting a repository that is not Questing and provides
Podman 5 or newer. Preserve the existing pinned action reference and ensure the
configured runner matches the repository.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a9bd5e48-4832-4a61-85a1-688423c1627d
📒 Files selected for processing (16)
.github/workflows/ansible-lint.yml.github/workflows/ansible-managed-var-comment.yml.github/workflows/ansible-test.yml.github/workflows/build_docs.yml.github/workflows/changelog_to_tag.yml.github/workflows/codeql.yml.github/workflows/codespell.yml.github/workflows/markdownlint.yml.github/workflows/pr-title-lint.yml.github/workflows/python-unit-test.yml.github/workflows/qemu-kvm-integration-tests.yml.github/workflows/shellcheck.yml.github/workflows/test_converting_readme.yml.github/workflows/tft.yml.github/workflows/weekly_ci.yml.github/workflows/woke.yml
The latest security guidance is to use the full commit hash, which is immutable, instead of a tag or version, which can be mutable, for the reference to a version of a github action. There are known attacks which inserted unauthorized code in a version tag and moved the tag. This prevents this sort of attack, at the cost of more maintenance burden, but dependabot will largely take care of this for us. Each version or tag has been replaced with the corresponding commit hash - in some cases, this is not the latest commit on the main branch, so I would expect to see some dependabot updates in the near future. I thought it was safer to do it this way - preserve existing behavior/functionality - rather than replace and upgrade to a newer version at the same time. The coderabbit `Pin the Galaxy action's container image.` issue is tracked at robertdebock/galaxy-action#16 This also adds `persist-credentials: false` to the actions/checkout tasks so that any credentials used by that task will not persist for subsequent tasks, for those workflows that do not need the credentials for subsequent tasks. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Contributor
Author
|
@coderabbitai review |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The latest security guidance is to use the full commit hash, which is immutable,
instead of a tag or version, which can be mutable, for the reference to a version
of a github action. There are known attacks which inserted unauthorized code
in a version tag and moved the tag. This prevents this sort of attack, at the
cost of more maintenance burden, but dependabot will largely take care of this
for us.
Each version or tag has been replaced with the corresponding commit hash - in some
cases, this is not the latest commit on the main branch, so I would expect to see
some dependabot updates in the near future. I thought it was safer to do it this
way - preserve existing behavior/functionality - rather than replace and upgrade
to a newer version at the same time.
The coderabbit
Pin the Galaxy action's container image.issue is tracked atrobertdebock/galaxy-action#16
This also adds
persist-credentials: falseto the actions/checkout tasks so thatany credentials used by that task will not persist for subsequent tasks, for those
workflows that do not need the credentials for subsequent tasks.
Summary by CodeRabbit