Fix overly broad permissions and template injection risks in GHA workflows#185
Conversation
…flows Signed-off-by: Caleb Xu <caxu@redhat.com>
WalkthroughThis PR hardens GitHub Actions workflow security by explicitly declaring read-only ChangesGitHub Actions Workflow Security and Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-main.yml:
- Around line 39-43: The workflow currently splits _IMAGE_REGISTRY with cut -d
'/' -f 1 and -f 2 which drops any nested repo path; change the parsing so
IMAGE_REGISTRY = first segment before the first '/' and IMAGE_REPO = everything
after the first '/' (i.e., take the remainder, not only the second segment) and
add validation to ensure _IMAGE_REGISTRY contains at least one '/' and non-empty
repo portion; update the commands that set IMAGE_REGISTRY/IMAGE_REPO in both
places where _IMAGE_REGISTRY is used (reference the environment var
_IMAGE_REGISTRY and outputs written to GITHUB_ENV for IMAGE_REGISTRY and
IMAGE_REPO) so nested namespaces are preserved and the workflow fails early on
invalid format.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d783aab8-c87e-4ada-bec6-dd7d14634540
📒 Files selected for processing (3)
.github/workflows/build-main.yml.github/workflows/build-release.yml.github/workflows/ci.yml
| echo "IMAGE_REGISTRY=$(echo "${_IMAGE_REGISTRY}" | cut -d '/' -f 1)" >> "${GITHUB_ENV}" | ||
| echo "IMAGE_REPO=$(echo "${_IMAGE_REGISTRY}" | cut -d '/' -f 2)" >> "${GITHUB_ENV}" | ||
| env: | ||
| _IMAGE_REGISTRY: ${{ secrets.IMAGE_REGISTRY }} | ||
|
|
There was a problem hiding this comment.
Root cause: registry parsing truncates namespace in both workflow files.
Both .github/workflows/build-main.yml and .github/workflows/build-release.yml derive IMAGE_REPO with cut -d '/' -f 2, which loses nested path segments and can push to unintended repositories. Use “first segment as registry, remainder as repo” parsing in both files, with format validation.
🤖 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/build-main.yml around lines 39 - 43, The workflow
currently splits _IMAGE_REGISTRY with cut -d '/' -f 1 and -f 2 which drops any
nested repo path; change the parsing so IMAGE_REGISTRY = first segment before
the first '/' and IMAGE_REPO = everything after the first '/' (i.e., take the
remainder, not only the second segment) and add validation to ensure
_IMAGE_REGISTRY contains at least one '/' and non-empty repo portion; update the
commands that set IMAGE_REGISTRY/IMAGE_REPO in both places where _IMAGE_REGISTRY
is used (reference the environment var _IMAGE_REGISTRY and outputs written to
GITHUB_ENV for IMAGE_REGISTRY and IMAGE_REPO) so nested namespaces are preserved
and the workflow fails early on invalid format.
Summary by CodeRabbit
Chores