Refactor release workflows with reusable GitHub Actions#55
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughAdds a suite of new local composite GitHub Actions for release automation (version validation, Makefile updates, branch/tag creation, release notes), updates existing release workflows to use those actions, and adds a new sector-release workflow that composes the actions into a two-job release flow. ChangesRelease automation and workflow orchestration
sequenceDiagram
autonumber
actor User
participant Workflow as GitHub Workflow
participant Prep as prepare-release-branch (local action)
participant TagPrep as prepare-release-tag (local action)
participant Git as Git (checkout, tag, branch)
participant Makefile as Makefile (repo)
participant GH_API as GitHub API / softprops
User->>Workflow: trigger (manual or push)
Workflow->>Prep: run (version, push-branch, expiry)
Prep->>Validate: validate-version
Note right of Prep: validate-version, create-release-branch, update-makefile-version, update-makefile-expiry
Prep->>Git: create/check branch & push (optional)
Prep->>Makefile: update VERSION / expiry
Workflow->>TagPrep: run create-release steps (tag, notes)
TagPrep->>Git: check-tag-exists / create-and-push-tag
TagPrep->>Makefile: read version
TagPrep->>GH_API: create release (generate-release-notes -> GH API)
GH_API-->>Workflow: release created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yaml (1)
17-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe
pull_request: closedcase is handled correctly;workflow_dispatchcase lacks a ref constraint.For
pull_requestevents where the PR was merged (checked by themerged == truecondition), GitHub Actions correctly setsgithub.ref_nameto the target branch name—in this case, the release branch that the PR was merged into. The checkout will work as intended.However, for
workflow_dispatchtriggers,github.ref_namedefaults to the branch the workflow file was dispatched from, which is typically the default branch, not arelease-v*branch. The workflow will checkout and tag the wrong commit. To restrict dispatch to release branches, add an input parameter:on: workflow_dispatch: inputs: release_branch: description: 'Release branch (release-v*)' required: true type: choice options: - release-v1.0 - release-v1.1Then use
ref: ${{ inputs.release_branch }}in the checkout step (and fallback togithub.ref_namefor thepull_requestcase).🤖 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.yaml around lines 17 - 22, The workflow_dispatch trigger lacks a constrained ref so the Checkout step (uses: actions/checkout@v4, ref: ${{ github.ref_name }}) can checkout the wrong branch; add a required workflow_dispatch input (e.g., inputs.release_branch) that restricts choices to release-v* branches and update the Checkout step to use ref: ${{ inputs.release_branch }} with a fallback to github.ref_name for the pull_request/merged path so dispatches must specify the release branch while PR merges continue to use github.ref_name.
🧹 Nitpick comments (5)
.github/actions/detect-prerelease/action.yaml (1)
19-19: 💤 Low valuePrerelease regex is overly permissive.
-(alpha|beta|rc)will match anywhere in the version (e.g.1.0.0-alphabet-build,1.0.0+build-rc-meta). For semver-style prereleases you usually want the token to follow-immediately and be terminated by.,+, or end-of-string. Consider anchoring with a word boundary, e.g.-(alpha|beta|rc)(\.|\+|$).🤖 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/actions/detect-prerelease/action.yaml at line 19, The prerelease detection regex in the if conditional using the VERSION variable (if [[ "$VERSION" =~ -(alpha|beta|rc) ]]; then) is too permissive; update the pattern to require the prerelease token to follow '-' and be terminated by a dot, plus, or end-of-string (for example change the match to -(alpha|beta|rc)(\.|\+|$)) so only proper semver-style prereleases are detected; locate and replace the regex in the if [[ "$VERSION" =~ ... ]] conditional in action.yaml..github/actions/prepare-release-tag/action.yaml (1)
13-15: 💤 Low value
tag-createdreports intent rather than outcome.The output is computed solely from
steps.check-tag.outputs.exists == 'false', so it istruewhenever the tag was missing — even if theCreate and push tagstep subsequently failed or was skipped. If consumers rely on this to gate downstream behaviour (release creation, notifications), consider either giving the create step anidand basing the output on itsoutcome/conclusion, or renaming the output totag-was-missingto reflect what is actually being measured.🤖 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/actions/prepare-release-tag/action.yaml around lines 13 - 15, The output `tag-created` currently reflects only `steps.check-tag.outputs.exists == 'false'` (i.e. whether the tag was missing) rather than the actual creation result; either change this to derive from the "Create and push tag" step's result by giving that step an `id` (e.g. `create-tag`) and setting `tag-created` to check `steps.create-tag.outcome` or `steps.create-tag.conclusion`, or rename the output to `tag-was-missing` to accurately describe its current meaning (`value: ${{ steps.check-tag.outputs.exists == 'false' }}`) and add a new `tag-created` output based on the create step outcome if you need the true outcome for downstream gates..github/workflows/sector-release.yaml (1)
38-46: ⚖️ Poor tradeoffConsider moving the commit/push into the composite action.
The
prepare-release-branchaction creates the branch and rewrites the Makefile, but leaves the resulting working-tree changes uncommitted, forcing every consumer (this workflow today, potentially others later) to re-implement the samegit config+git commit+git pushboilerplate. Rolling that responsibility into the action — perhaps gated on the existingpush-branchinput — would make the behaviour symmetric with theautomated-release.yamlflow (wherepeter-evans/create-pull-requesthandles the commit) and remove duplication.🤖 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/sector-release.yaml around lines 38 - 46, Move the commit/push boilerplate into the prepare-release-branch composite action so consumers don't repeat it: update the composite action (prepare-release-branch) to perform git config user.email and user.name, run git commit with the same message format ("Prepare release ${{ github.event.inputs.version }}") and push to the branch name produced as outputs.branch-name, and make this behavior conditional on the composite's existing push-branch input (only commit & push when push-branch is true); then remove the "Commit and push changes" step from the workflow..github/actions/create-release/action.yaml (2)
15-26: ⚡ Quick winConsider refactoring to use action outputs instead of relying on environment variables.
The
Generate release notesstep implicitly setsenv.releaseBodyviaGITHUB_ENVin the bash script it runs (lines 31-33 of./utils/release/github-release-changelog.sh). Whilst this currently works, the contract is not explicit in the action definition.Refactor the
generate-release-notesaction to define an output and write toGITHUB_OUTPUTinstead. This would allow theCreate GitHub Releasestep to consumesteps.notes.outputs.body, making the data flow explicit and far easier to audit.🤖 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/actions/create-release/action.yaml around lines 15 - 26, Refactor the generate-release-notes action (./.github/actions/generate-release-notes and its script ./utils/release/github-release-changelog.sh) to emit the changelog via a defined action output instead of writing to GITHUB_ENV: modify the script to write the release body to GITHUB_OUTPUT (e.g., "body=<content>" >> "$GITHUB_OUTPUT") and add an outputs entry in the action metadata so the action exposes output "body"; then update the workflow steps (the "Generate release notes" step and the "Create GitHub Release" step) to consume that output (use steps.generate-release-notes.outputs.body for the softprops/action-gh-release body and avoid relying on env.releaseBody).
21-30: ⚡ Quick winPin
softprops/action-gh-releaseto a commit SHA.Floating major tags (
@v2) can be force-moved by the upstream maintainer, which is a supply-chain risk for a release-publishing action that runs with a privileged PAT. Pinning to a full commit SHA (with the version as a comment) is the GitHub-recommended hardening for third-party actions in release-critical paths.🤖 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/actions/create-release/action.yaml around lines 21 - 30, Replace the floating action reference softprops/action-gh-release@v2 with a pinned commit SHA: update the uses: line in the Create GitHub Release step to use the full commit SHA for softprops/action-gh-release (keeping “@v2” as a comment for clarity), preserving all existing inputs (name, tag_name, body, target_commitish, prerelease) and the GITHUB_TOKEN env mapping; ensure the SHA points to a commit on the action repo and add a short inline comment noting the original `@v2` version.
🤖 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/actions/check-tag-exists/action.yaml:
- Around line 18-19: The step currently assigns TAG="${{ inputs.tag }}" directly
into the bash script which allows script injection; change the action to pass
the tag via the env block (e.g., set env: TAG: ${{ inputs.tag }} and
CURRENT_SHA: ${{ github.sha }}) and then reference $TAG inside the run script so
the value is treated as data; update the action.yaml step that defines
TAG/CURRENT_SHA to use env instead of inline interpolation and ensure any use of
TAG in the run section is quoted (e.g., "$TAG") to avoid word-splitting.
In @.github/actions/create-and-push-tag/action.yaml:
- Around line 16-19: The step currently embeds the expression `${{ inputs.tag
}}` directly into the shell which can enable script injection; change the step
to set the input into an environment variable (e.g. create env: TAG: ${{
inputs.tag }}) and then reference that env var inside the shell script (use
"$TAG" for the tag and in the git tag/git push commands) so the value is treated
as data; update occurrences that set TAG and the lines running git tag -a and
git push to use the env-provided TAG.
In @.github/actions/create-release-branch/action.yaml:
- Around line 25-41: The workflow exposes a script-injection risk by
template-expanding inputs.version and inputs.push directly into the shell; move
both ${{ inputs.version }} and ${{ inputs.push }} into environment variables
(e.g., VERSION and PUSH) and use those env vars inside the script,
sanitize/strip dangerous chars if needed, and ensure all branch uses of
base_branch (the variable computed in the script) are quoted (e.g., where git
checkout, git checkout -b and git push reference $base_branch) to avoid
word-splitting or injection; update the logic that computes base_branch so it
reads from VERSION, and change the conditional that compares PUSH to "true" to
reference the PUSH env var.
In @.github/actions/create-release/action.yaml:
- Around line 3-7: The action lacks an inputs declaration for the release branch
(and optionally tag) referenced later; add an inputs entry named release-branch
(and optionally tag) to the inputs: block so callers can supply it, and update
usages that set target_commitish to read from inputs.release-branch (or fallback
to github.ref_name) in the action metadata; alternatively ensure the
prepare-release-tag step or the workflows (.github/workflows/release.yaml and
sector-release.yaml) pass the new inputs.release-branch value when invoking this
action so softprops/action-gh-release receives a non-empty target_commitish.
In @.github/actions/detect-prerelease/action.yaml:
- Around line 17-23: Avoid direct interpolation of ${{ inputs.version }} into
the run block: move the input into the action environment (set an env variable
like VERSION from inputs.version) and reference it in the script as a quoted
shell variable ("$VERSION") instead of embedding the template, and also quote
"$GITHUB_OUTPUT" when appending output; update the run block that currently sets
VERSION="${{ inputs.version }}" and writes to $GITHUB_OUTPUT to instead read
from the env VERSION and use quoted variables to prevent shell injection.
In @.github/actions/get-version-from-makefile/action.yaml:
- Around line 17-20: The current extraction of VERSION into the VERSION variable
is fragile: modify the command that sets VERSION so it only captures a single
value (e.g., use grep -m1 or awk to take the first matching line and strip the
prefix) and trim whitespace, then validate that VERSION is non-empty and fail
fast if it is (emit a clear error and exit non-zero) before writing to
$GITHUB_OUTPUT; ensure subsequent writes use the validated single-line VERSION
for both "version" and "tag" to avoid malformed/multi-line outputs.
In @.github/actions/update-makefile-expiry/action.yaml:
- Around line 14-15: Move the input expiry into an env variable instead of
expanding `${{ inputs.expiry }}` inline (set QUAY_IMAGE_EXPIRY via an env block)
and then reference that env var in the run step; also harden the sed
substitution in the run command that updates the Makefile by switching the
delimiter from `/` to a different safe delimiter such as `|` (i.e., perform sed
-i "s|^QUAY_IMAGE_EXPIRY ?= .*|QUAY_IMAGE_EXPIRY ?= $QUAY_IMAGE_EXPIRY|") so the
replacement is not broken by values containing `/` while still using the
QUAY_IMAGE_EXPIRY env variable for substitution.
In @.github/actions/update-makefile-version/action.yaml:
- Around line 13-14: Replace direct expansion of ${{ inputs.version }} with
assigning it to an intermediate environment variable (e.g., VERSION) before any
shell execution to avoid GitHub expression injection; then update the sed
invocation that edits the Makefile (the line that currently runs sed -i
"s/^VERSION ?= .*/VERSION ?= $VERSION/") to use a safer delimiter such as | and
ensure the replacement side treats the VERSION variable literally (i.e., use a
quoted shell variable replacement with proper escaping/quoting) so that slashes,
ampersands, and backslashes in VERSION cannot break or corrupt the sed
substitution.
In @.github/actions/validate-version/action.yaml:
- Around line 13-16: The workflow currently assigns VERSION using the template
expansion VERSION="${{ inputs.version }}" which evaluates before shell execution
and risks command injection; change the action to pass the input into the shell
via an environment variable instead (e.g., map the input to an env like
INPUT_VERSION or VERSION in the action.yml/step and then in the script use
VERSION="$VERSION" or read from $INPUT_VERSION), then run the semver regex check
against that environment variable (the symbols to update are the VERSION
assignment and the regex check) so no template expansion happens inside the
shell command.
In @.github/workflows/sector-release.yaml:
- Around line 38-45: The job fails when running the "Commit and push changes"
step because git commit . exits non-zero if there are no staged changes; update
that step (the run block that calls git commit . -s -m "Prepare release ${{
github.event.inputs.version }}") to first check for changes (e.g., by testing
git status --porcelain or git diff --quiet) and only run git commit when there
are staged changes, or alternatively use git commit --allow-empty to make the
commit idempotent; adjust the step that references
steps.prepare-branch.outputs.branch-name accordingly so the push still occurs in
either case.
---
Outside diff comments:
In @.github/workflows/release.yaml:
- Around line 17-22: The workflow_dispatch trigger lacks a constrained ref so
the Checkout step (uses: actions/checkout@v4, ref: ${{ github.ref_name }}) can
checkout the wrong branch; add a required workflow_dispatch input (e.g.,
inputs.release_branch) that restricts choices to release-v* branches and update
the Checkout step to use ref: ${{ inputs.release_branch }} with a fallback to
github.ref_name for the pull_request/merged path so dispatches must specify the
release branch while PR merges continue to use github.ref_name.
---
Nitpick comments:
In @.github/actions/create-release/action.yaml:
- Around line 15-26: Refactor the generate-release-notes action
(./.github/actions/generate-release-notes and its script
./utils/release/github-release-changelog.sh) to emit the changelog via a defined
action output instead of writing to GITHUB_ENV: modify the script to write the
release body to GITHUB_OUTPUT (e.g., "body=<content>" >> "$GITHUB_OUTPUT") and
add an outputs entry in the action metadata so the action exposes output "body";
then update the workflow steps (the "Generate release notes" step and the
"Create GitHub Release" step) to consume that output (use
steps.generate-release-notes.outputs.body for the softprops/action-gh-release
body and avoid relying on env.releaseBody).
- Around line 21-30: Replace the floating action reference
softprops/action-gh-release@v2 with a pinned commit SHA: update the uses: line
in the Create GitHub Release step to use the full commit SHA for
softprops/action-gh-release (keeping “@v2” as a comment for clarity), preserving
all existing inputs (name, tag_name, body, target_commitish, prerelease) and the
GITHUB_TOKEN env mapping; ensure the SHA points to a commit on the action repo
and add a short inline comment noting the original `@v2` version.
In @.github/actions/detect-prerelease/action.yaml:
- Line 19: The prerelease detection regex in the if conditional using the
VERSION variable (if [[ "$VERSION" =~ -(alpha|beta|rc) ]]; then) is too
permissive; update the pattern to require the prerelease token to follow '-' and
be terminated by a dot, plus, or end-of-string (for example change the match to
-(alpha|beta|rc)(\.|\+|$)) so only proper semver-style prereleases are detected;
locate and replace the regex in the if [[ "$VERSION" =~ ... ]] conditional in
action.yaml.
In @.github/actions/prepare-release-tag/action.yaml:
- Around line 13-15: The output `tag-created` currently reflects only
`steps.check-tag.outputs.exists == 'false'` (i.e. whether the tag was missing)
rather than the actual creation result; either change this to derive from the
"Create and push tag" step's result by giving that step an `id` (e.g.
`create-tag`) and setting `tag-created` to check `steps.create-tag.outcome` or
`steps.create-tag.conclusion`, or rename the output to `tag-was-missing` to
accurately describe its current meaning (`value: ${{
steps.check-tag.outputs.exists == 'false' }}`) and add a new `tag-created`
output based on the create step outcome if you need the true outcome for
downstream gates.
In @.github/workflows/sector-release.yaml:
- Around line 38-46: Move the commit/push boilerplate into the
prepare-release-branch composite action so consumers don't repeat it: update the
composite action (prepare-release-branch) to perform git config user.email and
user.name, run git commit with the same message format ("Prepare release ${{
github.event.inputs.version }}") and push to the branch name produced as
outputs.branch-name, and make this behavior conditional on the composite's
existing push-branch input (only commit & push when push-branch is true); then
remove the "Commit and push changes" step from the workflow.
🪄 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
Run ID: a362c59f-b981-4e85-981e-45f48c65dfe1
📒 Files selected for processing (15)
.github/actions/check-tag-exists/action.yaml.github/actions/create-and-push-tag/action.yaml.github/actions/create-release-branch/action.yaml.github/actions/create-release/action.yaml.github/actions/detect-prerelease/action.yaml.github/actions/generate-release-notes/action.yaml.github/actions/get-version-from-makefile/action.yaml.github/actions/prepare-release-branch/action.yaml.github/actions/prepare-release-tag/action.yaml.github/actions/update-makefile-expiry/action.yaml.github/actions/update-makefile-version/action.yaml.github/actions/validate-version/action.yaml.github/workflows/automated-release.yaml.github/workflows/release.yaml.github/workflows/sector-release.yaml
| QUAY_IMAGE_EXPIRY="${{ inputs.expiry }}" | ||
| sed -i "s/^QUAY_IMAGE_EXPIRY ?= .*/QUAY_IMAGE_EXPIRY ?= $QUAY_IMAGE_EXPIRY/" Makefile |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "action.yaml" -path "*update-makefile-expiry*" | head -5Repository: Kuadrant/developer-portal-controller
Length of output: 133
🏁 Script executed:
cat -n .github/actions/update-makefile-expiry/action.yamlRepository: Kuadrant/developer-portal-controller
Length of output: 780
🌐 Web query:
GitHub Actions inputs expansion security script injection env block vs run
💡 Result:
In GitHub Actions, expressions like ${{ inputs.myinput }} or ${{ github.event.pull_request.title }} undergo macro expansion before execution, creating security risks for script injection when used directly in run blocks versus env blocks. Vulnerable pattern in run block (direct expansion leads to command injection): - run: echo "Title: ${{ github.event.issue.title }}" If title is a"; ls;#, it expands to echo "Title: a"; ls;#", executing ls [1][2]. Safe pattern using env block: - env: TITLE: ${{ github.event.issue.title }} run: echo "Title: $TITLE" The untrusted value is set as an environment variable (no expansion in shell script generation), treated as literal string in shell. Shell expansion of $TITLE is safe as it's data, not code structure [3][1][4][5][2]. This applies to workflow inputs (inputs context) and github.event contexts, both potentially untrusted if caller-controlled. Official docs warn against direct use in run/script [1][6]. For reusable workflows (workflow_call), use inputs context safely via env [7]. Recommendation: Always route untrusted expressions (inputs, github.event fields like title/body) through env before run. Further sanitize in scripts if needed [3][8].
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://securitylab.github.com/resources/github-actions-untrusted-input/
- 3: https://github.blog/2023-08-09-four-tips-to-keep-your-github-actions-workflows-secure
- 4: https://berkdede.medium.com/command-injection-by-design-when-github-actions-turns-metadata-into-code-aa644d396fb1
- 5: https://docs.github.com/en/actions/reference/security/secure-use
- 6: https://docs.github.com/en/actions/concepts/workflows-and-actions/expressions
- 7: https://stackoverflow.com/questions/71458189/github-action-workflow-call-does-not-use-up-to-date-input-values
- 8: https://actsense.dev/vulnerabilities/risky_context_usage/
🏁 Script executed:
# Check sed behavior with different delimiters and special characters
python3 << 'EOF'
import subprocess
import tempfile
import os
# Test case 1: sed with / delimiter and value containing /
test_cases = [
("never", "normal value", "/"),
("3w", "normal value", "/"),
("never/extra", "value with forward slash", "/"),
("never", "normal value", "|"),
]
for value, desc, delimiter in test_cases:
with tempfile.NamedTemporaryFile(mode='w', delete=False, suffix='.txt') as f:
f.write("QUAY_IMAGE_EXPIRY ?= old_value\n")
f.flush()
fname = f.name
try:
if delimiter == "/":
cmd = f'sed -i "s/^QUAY_IMAGE_EXPIRY ?= .*/QUAY_IMAGE_EXPIRY ?= {value}/" {fname}'
else:
cmd = f'sed -i "s|^QUAY_IMAGE_EXPIRY ?= .*|QUAY_IMAGE_EXPIRY ?= {value}|" {fname}'
result = subprocess.run(cmd, shell=True, capture_output=True, text=True)
with open(fname) as f:
content = f.read()
status = "✓ SUCCESS" if result.returncode == 0 else "✗ FAILED"
print(f"{status}: value={value}, delimiter={delimiter}")
print(f" Description: {desc}")
if result.returncode == 0:
print(f" Result: {content.strip()}")
else:
print(f" Error: {result.stderr.strip()}")
finally:
os.unlink(fname)
print()
EOFRepository: Kuadrant/developer-portal-controller
Length of output: 525
Script injection and sed delimiter vulnerability.
Two issues:
-
Script injection (Line 14):
${{ inputs.expiry }}expands directly in the run block before bash execution. Per GitHub Actions security documentation, this creates a script injection vector. Move the input to anenvblock to pass it safely as a literal string. -
sed delimiter fragility (Line 15): The sed command uses
/as the delimiter. Testing confirms this fails ifQUAY_IMAGE_EXPIRYcontains/(e.g., with inputnever/extra, sed errors with "unknown option to 's'"). Although documented values (never,3w) don't include/, there is no input validation. Switching to|as the delimiter makes the command robust for all cases.
🔒 Proposed fix
+ env:
+ INPUT_EXPIRY: ${{ inputs.expiry }}
run: |
- QUAY_IMAGE_EXPIRY="${{ inputs.expiry }}"
- sed -i "s/^QUAY_IMAGE_EXPIRY ?= .*/QUAY_IMAGE_EXPIRY ?= $QUAY_IMAGE_EXPIRY/" Makefile
+ QUAY_IMAGE_EXPIRY="$INPUT_EXPIRY"
+ sed -i "s|^QUAY_IMAGE_EXPIRY ?= .*|QUAY_IMAGE_EXPIRY ?= $QUAY_IMAGE_EXPIRY|" Makefile🤖 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/actions/update-makefile-expiry/action.yaml around lines 14 - 15,
Move the input expiry into an env variable instead of expanding `${{
inputs.expiry }}` inline (set QUAY_IMAGE_EXPIRY via an env block) and then
reference that env var in the run step; also harden the sed substitution in the
run command that updates the Makefile by switching the delimiter from `/` to a
different safe delimiter such as `|` (i.e., perform sed -i "s|^QUAY_IMAGE_EXPIRY
?= .*|QUAY_IMAGE_EXPIRY ?= $QUAY_IMAGE_EXPIRY|") so the replacement is not
broken by values containing `/` while still using the QUAY_IMAGE_EXPIRY env
variable for substitution.
| VERSION="${{ inputs.version }}" | ||
| sed -i "s/^VERSION ?= .*/VERSION ?= $VERSION/" Makefile |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the target file
find . -type f -name "action.yaml" -path "*update-makefile-version*" | head -20Repository: Kuadrant/developer-portal-controller
Length of output: 134
🏁 Script executed:
# Also search for update-makefile-expiry to compare
find . -type f -name "action.yaml" -path "*update-makefile-expiry*" | head -20Repository: Kuadrant/developer-portal-controller
Length of output: 133
🏁 Script executed:
# List all action.yaml files to understand directory structure
find . -type f -name "action.yaml" | head -30Repository: Kuadrant/developer-portal-controller
Length of output: 688
🏁 Script executed:
# Read the target file to verify the code at lines 13-14
cat -n ./.github/actions/update-makefile-version/action.yamlRepository: Kuadrant/developer-portal-controller
Length of output: 654
🏁 Script executed:
# Read the reference file for comparison
cat -n ./.github/actions/update-makefile-expiry/action.yamlRepository: Kuadrant/developer-portal-controller
Length of output: 780
🏁 Script executed:
# Check the validate-version action to see what validation it performs
cat -n ./.github/actions/validate-version/action.yamlRepository: Kuadrant/developer-portal-controller
Length of output: 785
🏁 Script executed:
# Look at how validate-version is used in workflows
find . -type f \( -name "*.yml" -o -name "*.yaml" \) -path "*/.github/workflows/*" | head -20Repository: Kuadrant/developer-portal-controller
Length of output: 355
🏁 Script executed:
# Search workflows to see how update-makefile-version is called
rg "update-makefile-version" ./.github/workflows/ -A 3 -B 3Repository: Kuadrant/developer-portal-controller
Length of output: 62
🏁 Script executed:
# Check which workflows use update-makefile-version and validate-version
rg -l "update-makefile-version" ./.github/workflows/Repository: Kuadrant/developer-portal-controller
Length of output: 62
🏁 Script executed:
# Broader search across entire repo
rg "update-makefile-version" . -lRepository: Kuadrant/developer-portal-controller
Length of output: 62
🏁 Script executed:
# Check if any workflow files reference it indirectly or in comments
rg "update.*makefile.*version" . -iRepository: Kuadrant/developer-portal-controller
Length of output: 62
🏁 Script executed:
# Also check for any documentation about these actions
find . -type f \( -name "README.md" -o -name "*.md" \) | xargs rg -l "makefile\|action" 2>/dev/null | head -10Repository: Kuadrant/developer-portal-controller
Length of output: 62
🌐 Web query:
GitHub Actions script injection context variable expansion security
💡 Result:
In GitHub Actions, “script injection” happens when untrusted context data is expanded into code executed by the runner (most commonly inside a run:/script: block). GitHub explicitly warns that expression values inside ${ {... } } are evaluated and substituted before the temporary shell script is executed, which can make ${ {... } } a shell command injection sink if the value contains characters that alter the script’s structure. GitHub also notes that attackers can add malicious content to the github context and that those values should be treated as potentially untrusted input [1][2]. Key context variable expansion risks (and where to look) 1) Using ${ {... } } directly inside run/script blocks - GitHub describes an inline-script vulnerability pattern: run executes within a temporary shell script, and expressions inside ${ { } } are evaluated and substituted before execution, enabling injection (example uses an attacker-controlled pull request title to inject a command) [1][2]. - GitHub’s security hardening guidance says the recommended approach is to avoid inline script sinks and instead pass context values as action arguments, or use intermediate environment variables for inline scripts [2]. 2) Environment file injection via GITHUB_ENV (and related) - Writing untrusted data into the special environment files (especially GITHUB_ENV) can let an attacker inject additional environment variables via newline injection or delimiters, which can ultimately enable arbitrary code execution depending on what environment variables affect subsequent steps/processes [3][4]. - GitHub Security Lab advisories show real impact: if a workflow reads attacker-controlled content and appends it into
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://docs.github.com/enterprise-server@3.0/actions/security-guides/security-hardening-for-github-actions
- 3: https://codeql.github.com/codeql-query-help/actions/actions-envvar-injection-medium/
- 4: https://codeql.github.com/codeql-query-help/actions/actions-envvar-injection-critical/
- 5: https://securitylab.github.com/advisories/GHSL-2024-255_docker-mailserver/
- 6: https://securitylab.github.com/advisories/GHSL-2024-294_Azure-cli/
- 7: https://securitylab.github.com/advisories/GHSL-2024-173_Feign/
- 8: https://docs.github.com/en/actions/reference/contexts-reference
- 9: https://github.blog/2023-08-09-four-tips-to-keep-your-github-actions-workflows-secure
- 10: https://docs.github.com/actions/learn-github-actions/security-hardening-for-github-actions
- 11: https://docs.github.com/en/enterprise-server@3.13/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions?learn=getting_started&learnProduct=actions
🏁 Script executed:
# Test sed vulnerability - does / character break the sed command?
python3 << 'EOF'
import subprocess
import tempfile
import os
# Create a temporary Makefile
with tempfile.TemporaryDirectory() as tmpdir:
makefile_path = os.path.join(tmpdir, "Makefile")
# Write a test Makefile
with open(makefile_path, 'w') as f:
f.write("VERSION ?= 1.0.0\n")
test_cases = [
("1.2.3", "normal semver"),
("1.2.3-alpha", "semver with prerelease"),
("1/2/3", "version with slashes"),
("1.2.3&test", "version with ampersand"),
("1.2.3\\test", "version with backslash"),
]
for version_str, description in test_cases:
# Read original
with open(makefile_path, 'r') as f:
original = f.read()
# Reset file
with open(makefile_path, 'w') as f:
f.write("VERSION ?= 1.0.0\n")
# Try the vulnerable sed command
cmd = f'sed -i "s/^VERSION ?= .*/VERSION ?= {version_str}/" {makefile_path}'
result = subprocess.run(cmd, shell=True, cwd=tmpdir, capture_output=True, text=True)
# Read result
with open(makefile_path, 'r') as f:
result_content = f.read()
print(f"\n{description}: '{version_str}'")
print(f"Command exit code: {result.returncode}")
if result.stderr:
print(f"Stderr: {result.stderr}")
print(f"Result content:\n{result_content}")
# Reset for next iteration
with open(makefile_path, 'w') as f:
f.write("VERSION ?= 1.0.0\n")
EOFRepository: Kuadrant/developer-portal-controller
Length of output: 618
Script injection and sed delimiter vulnerability — fix before use.
-
Script injection (Line 13):
${{ inputs.version }}expands directly into bash before execution. GitHub's security guidance explicitly identifies this as a vulnerable pattern. Use an intermediate environment variable instead. -
sed delimiter fragility (Line 14):
/is the delimiter; a version string containing/breaks sed entirely (exit code 1), while&and\cause silent corruption or misinterpretation. Use|as the delimiter.
Although validate-version constrains input to semver, update-makefile-version is a standalone action that can be called independently without validation.
Proposed fix
+ env:
+ INPUT_VERSION: ${{ inputs.version }}
run: |
- VERSION="${{ inputs.version }}"
- sed -i "s/^VERSION ?= .*/VERSION ?= $VERSION/" Makefile
+ VERSION="$INPUT_VERSION"
+ sed -i "s|^VERSION ?= .*|VERSION ?= $VERSION|" Makefile📝 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.
| VERSION="${{ inputs.version }}" | |
| sed -i "s/^VERSION ?= .*/VERSION ?= $VERSION/" Makefile | |
| env: | |
| INPUT_VERSION: ${{ inputs.version }} | |
| run: | | |
| VERSION="$INPUT_VERSION" | |
| sed -i "s|^VERSION ?= .*|VERSION ?= $VERSION|" Makefile |
🤖 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/actions/update-makefile-version/action.yaml around lines 13 - 14,
Replace direct expansion of ${{ inputs.version }} with assigning it to an
intermediate environment variable (e.g., VERSION) before any shell execution to
avoid GitHub expression injection; then update the sed invocation that edits the
Makefile (the line that currently runs sed -i "s/^VERSION ?= .*/VERSION ?=
$VERSION/") to use a safer delimiter such as | and ensure the replacement side
treats the VERSION variable literally (i.e., use a quoted shell variable
replacement with proper escaping/quoting) so that slashes, ampersands, and
backslashes in VERSION cannot break or corrupt the sed substitution.
| - name: Commit and push changes | ||
| shell: bash | ||
| run: | | ||
| git config --global user.email "${{ github.actor_id }}+${{ github.actor }}@users.noreply.github.com" | ||
| git config --global user.name "${{ github.actor}}" | ||
|
|
||
| git commit . -s -m "Prepare release ${{ github.event.inputs.version }}" | ||
| git push --set-upstream origin "${{ steps.prepare-branch.outputs.branch-name }}" |
There was a problem hiding this comment.
git commit . will fail the job when there is nothing to commit.
If prepare-release-branch produces no diff (e.g. the branch already existed at the requested version, or Makefile values already match), git commit . -s -m "..." exits non-zero and the job fails. Guard with a check, or fall back to --allow-empty, so re-running the workflow is idempotent.
🛡️ Proposed fix
- name: Commit and push changes
shell: bash
run: |
git config --global user.email "${{ github.actor_id }}+${{ github.actor }}@users.noreply.github.com"
git config --global user.name "${{ github.actor}}"
- git commit . -s -m "Prepare release ${{ github.event.inputs.version }}"
- git push --set-upstream origin "${{ steps.prepare-branch.outputs.branch-name }}"
+ if git diff --quiet && git diff --cached --quiet; then
+ echo "No changes to commit; pushing branch as-is."
+ else
+ git commit . -s -m "Prepare release ${{ github.event.inputs.version }}"
+ fi
+ git push --set-upstream origin "${{ steps.prepare-branch.outputs.branch-name }}"🤖 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/sector-release.yaml around lines 38 - 45, The job fails
when running the "Commit and push changes" step because git commit . exits
non-zero if there are no staged changes; update that step (the run block that
calls git commit . -s -m "Prepare release ${{ github.event.inputs.version }}")
to first check for changes (e.g., by testing git status --porcelain or git diff
--quiet) and only run git commit when there are staged changes, or alternatively
use git commit --allow-empty to make the commit idempotent; adjust the step that
references steps.prepare-branch.outputs.branch-name accordingly so the push
still occurs in either case.
This commit addresses critical security issues identified by coderabbitai in PR #55 review comments. Security Fixes: - Fix script injection vulnerabilities in 8 composite actions by moving template expressions (${{ inputs.* }}) from bash script bodies to environment variables - Add missing release-branch input declaration in create-release action - Improve Makefile parsing robustness with validation and error handling Changes: 1. Script Injection Prevention (Critical/Major): - check-tag-exists: Secure inputs.tag and github.sha - create-and-push-tag: Secure inputs.tag - create-release-branch: Secure inputs.version and inputs.push - detect-prerelease: Secure inputs.version - update-makefile-expiry: Secure inputs.expiry - update-makefile-version: Secure inputs.version - validate-version: Secure inputs.version - All bash variables properly quoted to prevent word-splitting 2. Missing Input Declaration (Critical): - create-release: Add release-branch input - release.yaml: Pass github.ref_name as release-branch - sector-release.yaml: Pass RELEASE_BRANCH output as release-branch 3. Fragile Parsing Fix (Minor): - get-version-from-makefile: Use grep -m1 to capture first match only - Add validation to ensure VERSION is non-empty - Add error handling with proper exit codes All changes prevent potential command injection attacks where untrusted input could be executed as shell commands through GitHub Actions template expansion. Reported-by: coderabbitai on PR #55 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Create a workflow to be used with the sector release tool. Main change is breaking the workflows up into reusable actions. Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
This commit addresses critical security issues identified by coderabbitai in PR #55 review comments. Security Fixes: - Fix script injection vulnerabilities in 8 composite actions by moving template expressions (${{ inputs.* }}) from bash script bodies to environment variables - Add missing release-branch input declaration in create-release action - Improve Makefile parsing robustness with validation and error handling Changes: 1. Script Injection Prevention (Critical/Major): - check-tag-exists: Secure inputs.tag and github.sha - create-and-push-tag: Secure inputs.tag - create-release-branch: Secure inputs.version and inputs.push - detect-prerelease: Secure inputs.version - update-makefile-expiry: Secure inputs.expiry - update-makefile-version: Secure inputs.version - validate-version: Secure inputs.version - All bash variables properly quoted to prevent word-splitting 2. Missing Input Declaration (Critical): - create-release: Add release-branch input - release.yaml: Pass github.ref_name as release-branch - sector-release.yaml: Pass RELEASE_BRANCH output as release-branch 3. Fragile Parsing Fix (Minor): - get-version-from-makefile: Use grep -m1 to capture first match only - Add validation to ensure VERSION is non-empty - Add error handling with proper exit codes All changes prevent potential command injection attacks where untrusted input could be executed as shell commands through GitHub Actions template expansion. Reported-by: coderabbitai on PR #55 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
1 participant
Summary
This PR refactors the release automation workflows by extracting inline bash scripts into reusable GitHub composite actions, improving maintainability and adding a new Sector-specific release workflow.
Changes
New Composite Actions
Created 12 reusable GitHub composite actions under
.github/actions/:Workflow Updates
Benefits
Testing
Summary by CodeRabbit