ci: use commit hash for github action, add persist-credentials false [citest_skip] - #896
Closed
richm wants to merge 2 commits into
Closed
ci: use commit hash for github action, add persist-credentials false [citest_skip]#896richm 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. 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. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
📝 WalkthroughWalkthroughChangesThe workflows replace mutable GitHub Actions references and selected CI dependency pinning
Possibly related PRs
🚥 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: 4
🧹 Nitpick comments (1)
.github/workflows/changelog_to_tag.yml (1)
82-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider replacing the superfluous release action.
zizmorreportsncipollo/release-actionas a superfluous action. This step has no output consumer in the workflow. A token-authenticatedgh release createstep can preserve the tag, title, notes file, and latest-release behavior while removing one third-party action. Verify--verify-tag,--notes-file,--title,--latest, and token permissions before changing it. GitHub CLI supports these options. (cli.github.com)🤖 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 with a token-authenticated GitHub CLI release creation step, preserving the existing tag, title, notes file, and latest-release behavior. Configure the command with --verify-tag, --notes-file, --title, and --latest, and ensure the workflow grants the token permissions required to create releases.Sources: MCP tools, 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 workflow’s actions/checkout configuration so the
docs-branch initialization push in the surrounding git flow is authenticated:
either enable checkout credential persistence or explicitly configure origin
with secrets.GITHUB_TOKEN before the push. Preserve the existing branch creation
and push behavior.
In @.github/workflows/changelog_to_tag.yml:
- Line 90: Update the robertdebock/galaxy-action invocation in the workflow to
use a publisher or action revision whose Docker container image is pinned by
immutable digest rather than the mutable latest tag, while preserving the
existing galaxy_api_key input and action behavior.
In @.github/workflows/weekly_ci.yml:
- Around line 33-35: Update the checkout step using actions/checkout to preserve
Git authentication for the later git push: remove persist-credentials: false, or
explicitly configure masked Git credentials before the push. Keep the existing
commit identity configuration unchanged.
In @.github/workflows/woke.yml:
- Around line 18-20: Update the “Run lsr-woke-action” workflow step so the
downloaded woke executable from the nhosoi/woke source is pinned to an immutable
commit or verified by checksum, rather than following main. Preserve the
existing action pin while configuring its downloaded artifact reference
accordingly.
---
Nitpick comments:
In @.github/workflows/changelog_to_tag.yml:
- Line 82: Replace the ncipollo/release-action step with a token-authenticated
GitHub CLI release creation step, preserving the existing tag, title, notes
file, and latest-release behavior. Configure the command with --verify-tag,
--notes-file, --title, and --latest, and ensure the workflow grants the token
permissions required to create releases.
🪄 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: f0d09290-4884-403d-af93-3eedc22647cf
📒 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
richm
changed the title
…[citest_skip] 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>
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.