Add two-phase release workflow - #634
Conversation
📝 WalkthroughWalkthroughAdds a two-phase release automation system comprising version parsing and validation scripts, pre-release and release workflows, Version Gate validation, reusable image-build inputs, a version seed file, and updated release documentation. ChangesTwo-phase release automation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant PreRelease as Pre-release workflow
participant VersionGate as Version Gate workflow
participant Release as Release workflow
participant BuildImages as Build-images workflow
participant GitHub as GitHub
Developer->>PreRelease: dispatch version and source branch
PreRelease->>GitHub: create release branch
PreRelease->>GitHub: push pre-release branch and open pull request
GitHub->>VersionGate: send qualifying pull request event
VersionGate->>VersionGate: validate release.yaml
Developer->>Release: dispatch release branch
Release->>GitHub: run checks and push version tag
Release->>BuildImages: call workflow with version and tag
BuildImages->>GitHub: build and push image
Release->>GitHub: create GitHub Release
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 |
5ffa6f0 to
f17c4d8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/scripts/validate-release-yaml.sh:
- Around line 20-24: The yq query on the line that assigns dep_version uses dot
notation with `.dependencies.${dep}`, which fails when dependency names contain
hyphens because yq interprets hyphens as subtraction operators rather than
literal characters. Replace the dot notation syntax with bracket notation by
changing `.dependencies.${dep}` to `.dependencies["${dep}"]` to properly handle
dependency names with special characters like hyphens.
In @.github/workflows/build-images.yaml:
- Around line 39-42: The conditional block starting at line 39 directly
interpolates the workflow input `${{ inputs.version }}` into shell code, which
creates a shell injection vulnerability if a caller passes values containing
shell metacharacters like `$(command)`. Instead of directly embedding the input
in the bash conditional and echo commands, set the input value as an environment
variable first, then reference that variable in the conditional check and the
subsequent echo statements that write to GITHUB_OUTPUT. This prevents the shell
from interpreting any special characters in the input value.
In @.github/workflows/pre-release.yaml:
- Line 27: The workflow file uses mutable version tags (such as `@v4` and `@v5`) for
GitHub Actions references on lines 27, 62, 82, and 108, which creates security
risks since these tags can be retargeted upstream. Replace each mutable version
tag with a specific immutable commit SHA. For the actions/checkout action on
line 27 and any other action references throughout the file, determine the
correct commit SHA for each action version (for example, `@v4` should be replaced
with the full commit SHA like `@e2f20f853a2a19803ed141a6f266e17681910f6d`) and
update all four occurrences to use the pinned commit hash instead of the version
tag.
In @.github/workflows/release.yaml:
- Around line 11-13: The `contents: write` permission is currently set at the
workflow level, granting write access to all jobs. Move this permission to only
the specific job(s) that create tags or releases by removing `contents: write`
from the workflow-level permissions block and adding it as a job-level
permission to only the job(s) that perform release operations. This ensures
other jobs in the workflow operate with minimal necessary privileges.
- Around line 91-94: The tag existence check in the release workflow is not
idempotent - it fails unconditionally whenever the tag already exists, blocking
recovery from partial failures. Modify the git tag validation logic to check if
the tag exists at the current HEAD commit rather than just checking if the tag
exists. If the tag already points to HEAD, allow the workflow to proceed as
success. Only exit with an error if the tag exists but points to a different
commit, indicating a genuine conflict.
- Around line 22-24: Replace all mutable GitHub Actions version tags in the
release.yaml workflow with pinned commit SHAs. Specifically, replace the `@v4`
tag in the actions/checkout action on line 22, and locate the other GitHub
Actions usages at lines 60, 65, 82, and 112, then replace their mutable version
tags (such as `@v4` or `@v5`) with their full commit SHAs. You can find the correct
commit SHAs by looking up each action's releases on GitHub or using the GitHub
CLI. This ensures that the workflow uses immutable action references and
prevents silent upstream changes that could affect supply-chain security.
- Around line 38-43: The step "Validate branch matches version" directly
interpolates the workflow input `inputs.release-branch` into the bash script,
creating a shell injection vulnerability. Move `inputs.release-branch` to an
environment variable by adding an `env` block to the step that sets a variable
(e.g., `RELEASE_BRANCH: ${{ inputs.release-branch }}`), then update the bash
script to reference this environment variable using native shell syntax (e.g.,
`$RELEASE_BRANCH`) instead of the direct `${{ inputs.release-branch }}`
interpolation on lines 41-42.
In @.github/workflows/version-gate.yaml:
- Around line 28-30: The validate-release-yaml.sh script is currently receiving
the source branch via github.head_ref, but it should receive the target branch
via github.base_ref to properly validate against the correct release branch.
Additionally, to prevent shell-template injection attacks through crafted branch
names, pass the branch reference as an environment variable rather than directly
interpolating it in the shell command. Modify the run block to export
github.base_ref as an environment variable and pass that variable to the
validate-release-yaml.sh script instead of the direct interpolation.
🪄 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: 9c0cdb3d-3870-4fdc-ae44-d790d6b496b1
📒 Files selected for processing (8)
.github/scripts/parse-version.sh.github/scripts/validate-release-yaml.sh.github/workflows/build-images.yaml.github/workflows/pre-release.yaml.github/workflows/release.yaml.github/workflows/version-gate.yamlRELEASE.mdrelease.yaml
| DEPS=$(yq '.dependencies | keys | .[]' "$RELEASE_YAML" 2>/dev/null || true) | ||
| for dep in $DEPS; do | ||
| dep_version=$(yq ".dependencies.${dep}" "$RELEASE_YAML") | ||
| if [[ "$dep_version" != "0.0.0" && "$dep_version" != "null" && -n "$dep_version" ]]; then | ||
| if ! gh release view "v${dep_version}" --repo "${ORG}/${dep}" &>/dev/null; then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the validate-release-yaml.sh file
git ls-files | grep -i "validate-release-yaml"Repository: Kuadrant/authorino
Length of output: 197
🏁 Script executed:
# Read the file to see the current state
cat -n .github/scripts/validate-release-yaml.sh | head -40Repository: Kuadrant/authorino
Length of output: 1386
🏁 Script executed:
# Test yq behaviour with keys containing hyphens
# Create a test YAML to verify the concern
cat > /tmp/test-release.yaml <<'EOF'
dependencies:
my-service: "1.0.0"
another-component: "2.0.0"
simple: "3.0.0"
EOF
# Test current approach (without quotes)
echo "=== Testing current approach (unquoted key) ==="
yq '.dependencies | keys | .[]' /tmp/test-release.yaml
echo ""
echo "=== Testing current approach with unquoted variable ==="
dep="my-service"
yq ".dependencies.${dep}" /tmp/test-release.yaml 2>&1 || echo "FAILED"
echo ""
echo "=== Testing proposed fix with bracket notation ==="
yq -r ".dependencies[\"${dep}\"]" /tmp/test-release.yamlRepository: Kuadrant/authorino
Length of output: 435
🏁 Script executed:
# Check if there are any RELEASE_YAML files in the repo and their content
find . -name "*release*.yaml" -o -name "*release*.yml" | head -10Repository: Kuadrant/authorino
Length of output: 241
🌐 Web query:
yq key quoting hyphenated keys bracket notation syntax
💡 Result:
To access keys containing hyphens in yq, you must use bracket notation with quoted strings [1][2]. Because hyphenated keys can be misinterpreted by the parser as subtraction operations, wrapping the key name in quotes (either double or single) is necessary to ensure it is treated as a literal string [1][2]. The standard syntax for accessing such a key is: yq '.["your-hyphenated-key"]' file.yaml Alternatively, you can use the syntax: yq '."your-hyphenated-key"' file.yaml It is critical to use shell-level quoting (the outer single quotes) to prevent your shell from interpreting the brackets or quotes before they are passed to yq [1]. Common pitfalls: 1. Shell Interpretation: If you do not wrap the entire expression in single quotes (e.g., yq.["key-name"]), your shell may attempt to interpret the brackets or quotes, leading to errors. 2. Operator Confusion: If you omit quotes around the key name, yq may treat the hyphen as a subtraction operator rather than a character in the key name [1]. 3. Leading Dashes: If the key name itself starts with a hyphen, you may also need to use the flag terminator (--) to stop the command from misinterpreting the key as a command-line flag [3].
Citations:
- 1: How do I query for fields with dashes in them? kislyuk/yq#32
- 2: https://blog.hashhackers.com/blog/yq-guide/
- 3: https://mikefarah.gitbook.io/yq/v3.x/usage/path-expressions.md
🌐 Web query:
yq .dependencies.key-name vs .dependencies["key-name"] parsing difference
💡 Result:
In yq, the primary difference between dot notation (e.g.,.dependencies.key-name) and bracket notation (e.g.,.dependencies["key-name"]) is how they handle key names containing special characters, such as dots, hyphens, or spaces [1][2][3]. Dot notation is designed for simple, alphanumeric key names [4]. When a key contains special characters like a period, hyphen, or space, yq may misinterpret these characters as part of the expression's syntax—for example, treating a dot as a nested object separator rather than part of the key name [2][5][3]. Bracket notation with quotes (e.g.,.dependencies["key-name"]) acts as a robust mechanism to treat the contents within the brackets as a literal string [1][3]. This allows you to access keys that would otherwise be syntactically ambiguous or invalid in dot notation [2][6]. Key takeaways: 1. Dot notation: Suitable for standard, simple key names without special characters [4]. It cannot correctly reference keys containing dots (e.g.,.a.b.c interprets b.c as a nested structure) or other reserved symbols [2][5]. 2. Bracket notation: Required for keys containing dots, spaces, or leading dashes [1][2][3][7]. It is generally safer to use bracket notation whenever you are uncertain if a key name might conflict with yq expression syntax [6]. 3. Shell considerations: When using bracket notation in a terminal, it is important to wrap the entire expression in single quotes (e.g., '.dependencies["key-name"]') to prevent the shell from interpreting the double quotes or brackets prematurely [2][5][8]. For example, if you have a key named b.c, you must use.["b.c"] or."b.c" to access it, as.b.c would attempt to look for a key named b containing a sub-key named c [2][5].
Citations:
- 1: https://mikefarah.gitbook.io/yq/operators/traverse-read
- 2: https://mikefarah.gitbook.io/yq/v3.x/usage/path-expressions
- 3: Looking for a way to escape period for yaml params that have dots in them mikefarah/yq#2247
- 4: https://mikefarah.gitbook.io/yq/how-it-works
- 5: https://stackoverflow.com/questions/64983036/how-to-read-values-with-yq-when-the-key-has-a-dot
- 6: https://blog.hashhackers.com/blog/yq-guide/
- 7: https://mikefarah.gitbook.io/yq/v3.x/usage/path-expressions.md
- 8: How do I query for fields with dashes in them? kislyuk/yq#32
🏁 Script executed:
# Let me check the release.yaml file in the repo to see what dependencies actually exist
cat release.yaml 2>/dev/null | head -50Repository: Kuadrant/authorino
Length of output: 186
🏁 Script executed:
# Also check if there are any dependencies with hyphens in their names
# by examining any release YAML files
if [ -f release.yaml ]; then
echo "=== Content of release.yaml ==="
cat release.yaml
fiRepository: Kuadrant/authorino
Length of output: 218
Use bracket notation with quoted keys when querying yq for dynamic dependency names.
Line 22 currently builds .dependencies.${dep} using dot notation, which fails when dependency names contain hyphens. The yq parser misinterprets hyphens as subtraction operators rather than literal characters. This causes dependency version checks to fail silently or be skipped entirely for dependencies like my-service.
🔧 Proposed fix
-DEPS=$(yq '.dependencies | keys | .[]' "$RELEASE_YAML" 2>/dev/null || true)
-for dep in $DEPS; do
- dep_version=$(yq ".dependencies.${dep}" "$RELEASE_YAML")
+mapfile -t deps < <(yq -r '.dependencies // {} | keys[]' "$RELEASE_YAML")
+for dep in "${deps[@]}"; do
+ dep_version=$(yq -r ".dependencies[\"${dep}\"]" "$RELEASE_YAML")🤖 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/scripts/validate-release-yaml.sh around lines 20 - 24, The yq query
on the line that assigns dep_version uses dot notation with
`.dependencies.${dep}`, which fails when dependency names contain hyphens
because yq interprets hyphens as subtraction operators rather than literal
characters. Replace the dot notation syntax with bracket notation by changing
`.dependencies.${dep}` to `.dependencies["${dep}"]` to properly handle
dependency names with special characters like hyphens.
| if [[ -n "${{ inputs.version }}" ]]; then | ||
| echo "version=${{ inputs.version }}" >> $GITHUB_OUTPUT | ||
| echo "version_tag=v${{ inputs.version }}" >> $GITHUB_OUTPUT | ||
| elif [[ ${GITHUB_REF_NAME/\//-} =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/build-images.yaml | sed -n '1,50p'Repository: Kuadrant/authorino
Length of output: 2070
🏁 Script executed:
cat -n .github/workflows/build-images.yaml | sed -n '9,18p;75,76p;130,132p'Repository: Kuadrant/authorino
Length of output: 837
🏁 Script executed:
# Check if there are any uses of inputs.version elsewhere in the workflow
rg 'inputs\.version' .github/workflows/build-images.yamlRepository: Kuadrant/authorino
Length of output: 347
Harden workflow_call input handling before using it in bash.
Lines 39–41 interpolate ${{ inputs.version }} directly into shell code. If a caller passes a crafted value containing shell metacharacters (e.g. $(command)), the shell will expand it and execute arbitrary code, poisoning the workflow outputs. Pass the input via an environment variable instead to prevent expansion.
Suggested fix
- name: Set Authorino build info
id: build-info
+ env:
+ INPUT_VERSION: ${{ inputs.version }}
run: |
- if [[ -n "${{ inputs.version }}" ]]; then
- echo "version=${{ inputs.version }}" >> $GITHUB_OUTPUT
- echo "version_tag=v${{ inputs.version }}" >> $GITHUB_OUTPUT
+ if [[ -n "$INPUT_VERSION" ]]; then
+ if ! [[ "$INPUT_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then
+ echo "::error::Invalid version input: $INPUT_VERSION"
+ exit 1
+ fi
+ printf 'version=%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT"
+ printf 'version_tag=v%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT"
elif [[ ${GITHUB_REF_NAME/\/,-} =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then📝 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.
| if [[ -n "${{ inputs.version }}" ]]; then | |
| echo "version=${{ inputs.version }}" >> $GITHUB_OUTPUT | |
| echo "version_tag=v${{ inputs.version }}" >> $GITHUB_OUTPUT | |
| elif [[ ${GITHUB_REF_NAME/\//-} =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then | |
| - name: Set Authorino build info | |
| id: build-info | |
| env: | |
| INPUT_VERSION: ${{ inputs.version }} | |
| run: | | |
| if [[ -n "$INPUT_VERSION" ]]; then | |
| if ! [[ "$INPUT_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then | |
| echo "::error::Invalid version input: $INPUT_VERSION" | |
| exit 1 | |
| fi | |
| printf 'version=%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT" | |
| printf 'version_tag=v%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT" | |
| elif [[ ${GITHUB_REF_NAME/\/,-} =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 39-39: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 40-40: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 41-41: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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-images.yaml around lines 39 - 42, The conditional
block starting at line 39 directly interpolates the workflow input `${{
inputs.version }}` into shell code, which creates a shell injection
vulnerability if a caller passes values containing shell metacharacters like
`$(command)`. Instead of directly embedding the input in the bash conditional
and echo commands, set the input value as an environment variable first, then
reference that variable in the conditional check and the subsequent echo
statements that write to GITHUB_OUTPUT. This prevents the shell from
interpreting any special characters in the input value.
Source: Linters/SAST tools
| permissions: | ||
| contents: write | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reduce workflow token scope to least privilege.
contents: write at workflow scope grants write access to jobs that do not need it. Scope write permission only to jobs that create tags/releases.
Suggested fix
permissions:
- contents: write
+ contents: read
jobs:
+ tag:
+ permissions:
+ contents: write
+ needs: [read-version, smoke-tests]
+ runs-on: ubuntu-latest
+ steps:
+ ...
+
+ create-release:
+ permissions:
+ contents: write
+ needs: [read-version, build-image]
+ runs-on: ubuntu-latest
+ steps:
+ ...🧰 Tools
🪛 zizmor (1.26.1)
[error] 12-12: overly broad permissions (excessive-permissions): contents: write is overly broad at the workflow level
(excessive-permissions)
🤖 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 11 - 13, The `contents: write`
permission is currently set at the workflow level, granting write access to all
jobs. Move this permission to only the specific job(s) that create tags or
releases by removing `contents: write` from the workflow-level permissions block
and adding it as a job-level permission to only the job(s) that perform release
operations. This ensures other jobs in the workflow operate with minimal
necessary privileges.
Source: Linters/SAST tools
| - name: Validate branch matches version | ||
| run: | | ||
| EXPECTED_BRANCH="release-${{ steps.parse.outputs.major }}.${{ steps.parse.outputs.minor }}" | ||
| if [[ "${{ inputs.release-branch }}" != "$EXPECTED_BRANCH" ]]; then | ||
| echo "::error::Branch '${{ inputs.release-branch }}' does not match version ${{ steps.parse.outputs.version }} (expected branch: $EXPECTED_BRANCH)" | ||
| exit 1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the release.yaml file
git ls-files | grep -E 'release\.yaml|release\.yml'Repository: Kuadrant/authorino
Length of output: 235
🏁 Script executed:
# Check the content around lines 38-43 of the release.yaml file
cat -n .github/workflows/release.yaml | sed -n '30,50p'Repository: Kuadrant/authorino
Length of output: 1127
🌐 Web query:
GitHub Actions security best practices context interpolation shell script injection
💡 Result:
The primary risk of context interpolation in GitHub Actions arises because workflow expressions (the ${{ ... }} syntax) are evaluated by the GitHub Actions runner before the shell executes your script [1][2][3]. When you directly interpolate untrusted context values—such as github.event.issue.title or github.event.pull_request.head_ref—into a run step, you inadvertently allow an attacker to inject arbitrary shell commands [1][4][3]. To secure your workflows against this script injection vulnerability, follow these best practices: 1. Use Intermediate Environment Variables (Recommended) Never place ${{ ... }} expressions directly inside your shell command [5][6][3]. Instead, map the untrusted value to an environment variable in the env block of the step, and then reference that variable using the native shell syntax (e.g., $VAR or "$VAR") [7][5][3]. Vulnerable Example: run: echo "Processing PR: ${{ github.event.pull_request.title }}" Secure Example: env: PR_TITLE: ${{ github.event.pull_request.title }} run: echo "Processing PR: $PR_TITLE" 2. Avoid Re-interpolation Do not use ${{ env.VAR }} inside a run step to access an environment variable [5][6][3]. This re-introduces the interpolation vulnerability because the expression is evaluated before the shell executes the command [6][3]. Always use native shell syntax (e.g., $VAR) to access variables created in the env block [5][3]. 3. Use Actions for Complex Logic When possible, replace inline shell scripts with custom actions or existing community actions that process data as arguments rather than generating shell scripts [7][8]. Because arguments are passed directly to the action's code rather than being concatenated into a shell command string, this approach is inherently resistant to script injection [7][8]. 4. Treat All Contexts as Untrusted Any context that can be influenced by external users, such as PR titles, issue bodies, branch names, or author names, must be treated as untrusted input [1][4]. Be especially cautious with workflows triggered by events like pull_request, issue_comment, or workflow_run [8][3]. 5. Enable Security Scanning Use tools like CodeQL or ESLint plugins (e.g., eslint-plugin-github-actions) to automatically detect and flag unsafe interpolation of context values in your workflow YAML files [9][10]. Enabling code scanning helps prevent these vulnerabilities from being introduced in future changes [9].
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://phpunit.expert/articles/hardening-github-actions-workflows.html
- 3: https://cloud.hacktricks.wiki/en/pentesting-ci-cd/github-security/abusing-github-actions/gh-actions-context-script-injections.html
- 4: https://securitylab.github.com/resources/github-actions-untrusted-input/
- 5: https://github.com/github/codeql/blob/main/actions/ql/src/Security/CWE-094/CodeInjectionMedium.md
- 6: https://codeql.github.com/codeql-query-help/actions/actions-code-injection-critical/
- 7: https://docs.github.com/en/enterprise-server@2.22/actions/security-guides/security-hardening-for-github-actions
- 8: https://docs.github.com/en/actions/reference/security/secure-use
- 9: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
- 10: https://nick2bad4u.github.io/eslint-plugin-github-actions-2/docs/rules/no-untrusted-input-in-run/
🏁 Script executed:
# Check the workflow trigger and input definitions
cat -n .github/workflows/release.yaml | sed -n '1,30p'Repository: Kuadrant/authorino
Length of output: 1092
Pass inputs.release-branch via an environment variable to prevent shell injection.
Line 41–42 directly interpolates the workflow dispatch input into a bash script, which allows shell command injection if the input contains metacharacters. GitHub Actions evaluates ${{ ... }} expressions before the shell executes, making the input available for expansion. Pass the input through the env block and reference it with native shell syntax instead.
Suggested fix
- name: Validate branch matches version
+ env:
+ INPUT_RELEASE_BRANCH: ${{ inputs.release-branch }}
run: |
EXPECTED_BRANCH="release-${{ steps.parse.outputs.major }}.${{ steps.parse.outputs.minor }}"
- if [[ "${{ inputs.release-branch }}" != "$EXPECTED_BRANCH" ]]; then
- echo "::error::Branch '${{ inputs.release-branch }}' does not match version ${{ steps.parse.outputs.version }} (expected branch: $EXPECTED_BRANCH)"
+ if [[ "$INPUT_RELEASE_BRANCH" != "$EXPECTED_BRANCH" ]]; then
+ echo "::error::Branch '$INPUT_RELEASE_BRANCH' does not match version ${{ steps.parse.outputs.version }} (expected branch: $EXPECTED_BRANCH)"
exit 1
fi🧰 Tools
🪛 zizmor (1.26.1)
[info] 40-40: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 40-40: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 41-41: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 42-42: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 42-42: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 38 - 43, The step "Validate
branch matches version" directly interpolates the workflow input
`inputs.release-branch` into the bash script, creating a shell injection
vulnerability. Move `inputs.release-branch` to an environment variable by adding
an `env` block to the step that sets a variable (e.g., `RELEASE_BRANCH: ${{
inputs.release-branch }}`), then update the bash script to reference this
environment variable using native shell syntax (e.g., `$RELEASE_BRANCH`) instead
of the direct `${{ inputs.release-branch }}` interpolation on lines 41-42.
Source: Linters/SAST tools
| if git rev-parse "$TAG" >/dev/null 2>&1; then | ||
| echo "::error::Tag $TAG already exists" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make tag creation idempotent for safe reruns.
If a previous run pushed the tag but failed later, reruns will always fail here and block recovery. Treat “tag already exists at HEAD” as success.
Suggested fix
if git rev-parse "$TAG" >/dev/null 2>&1; then
- echo "::error::Tag $TAG already exists"
- exit 1
+ TAG_SHA="$(git rev-list -n1 "$TAG")"
+ HEAD_SHA="$(git rev-parse HEAD)"
+ if [[ "$TAG_SHA" != "$HEAD_SHA" ]]; then
+ echo "::error::Tag $TAG already exists and points to $TAG_SHA, not $HEAD_SHA"
+ exit 1
+ fi
+ echo "Tag $TAG already exists at HEAD; continuing"
+ exit 0
fi📝 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.
| if git rev-parse "$TAG" >/dev/null 2>&1; then | |
| echo "::error::Tag $TAG already exists" | |
| exit 1 | |
| fi | |
| if git rev-parse "$TAG" >/dev/null 2>&1; then | |
| TAG_SHA="$(git rev-list -n1 "$TAG")" | |
| HEAD_SHA="$(git rev-parse HEAD)" | |
| if [[ "$TAG_SHA" != "$HEAD_SHA" ]]; then | |
| echo "::error::Tag $TAG already exists and points to $TAG_SHA, not $HEAD_SHA" | |
| exit 1 | |
| fi | |
| echo "Tag $TAG already exists at HEAD; continuing" | |
| exit 0 | |
| fi |
🤖 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 91 - 94, The tag existence check
in the release workflow is not idempotent - it fails unconditionally whenever
the tag already exists, blocking recovery from partial failures. Modify the git
tag validation logic to check if the tag exists at the current HEAD commit
rather than just checking if the tag exists. If the tag already points to HEAD,
allow the workflow to proceed as success. Only exit with an error if the tag
exists but points to a different commit, indicating a genuine conflict.
| run: | | ||
| chmod +x .github/scripts/validate-release-yaml.sh | ||
| .github/scripts/validate-release-yaml.sh "${{ github.head_ref }}" "Kuadrant" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use github.base_ref via env to avoid injection and enforce the correct branch rule.
Lines 28-30 interpolate github.head_ref directly in the shell and pass the source branch, but the gate logic should evaluate the target release branch. This can both weaken validation semantics and allow shell-template injection through crafted PR branch names.
🔧 Proposed fix
- name: Validate release.yaml
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ BASE_REF: ${{ github.base_ref }}
run: |
chmod +x .github/scripts/validate-release-yaml.sh
- .github/scripts/validate-release-yaml.sh "${{ github.head_ref }}" "Kuadrant"
+ .github/scripts/validate-release-yaml.sh "$BASE_REF" "Kuadrant"📝 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.
| run: | | |
| chmod +x .github/scripts/validate-release-yaml.sh | |
| .github/scripts/validate-release-yaml.sh "${{ github.head_ref }}" "Kuadrant" | |
| - name: Validate release.yaml | |
| env: | |
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| BASE_REF: ${{ github.base_ref }} | |
| run: | | |
| chmod +x .github/scripts/validate-release-yaml.sh | |
| .github/scripts/validate-release-yaml.sh "$BASE_REF" "Kuadrant" |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 28-28: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🪛 zizmor (1.26.1)
[error] 30-30: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/version-gate.yaml around lines 28 - 30, The
validate-release-yaml.sh script is currently receiving the source branch via
github.head_ref, but it should receive the target branch via github.base_ref to
properly validate against the correct release branch. Additionally, to prevent
shell-template injection attacks through crafted branch names, pass the branch
reference as an environment variable rather than directly interpolating it in
the shell command. Modify the run block to export github.base_ref as an
environment variable and pass that variable to the validate-release-yaml.sh
script instead of the direct interpolation.
Source: Linters/SAST tools
| description: "Release version (semver, e.g. 1.5.0)" | ||
| required: true | ||
| type: string | ||
| source-branch: |
There was a problem hiding this comment.
Every workflow_dispatch already has a source branch field. Can't that one be used instead?
There was a problem hiding this comment.
Possible yes, but also maybe not. The scenario I am thinking that it is required is doing back ports to older releases. We will want to make sure that the workflow is ran from the branch that the backport is targeted at, but that would be different than the source-branch to run the workflow against.
Does that make sense?
There was a problem hiding this comment.
It makes sense.
For the workflow to run, I think it needs to be defined in the branch we pick in that field from the workflow_dispatch form. It would be hard to pre-release a patch on a version from when the workflow originally didn't exist yet. We would have to back port the workflow itself to all maintained versions.
Having the not-so-redundant field also gives us the option to run the latest version of the workflow (as in the one in main) against any arbitrary source branch, regardless of the version of the workflow in that branch.
I guess having to type the name of the branch again isn't the end of the world. We just need to be careful about typos – which arguably we could validate in a step of the workflow (if not already).
Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
f17c4d8 to
94b718d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.github/workflows/version-gate.yaml (1)
28-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse
github.base_refvia env to avoid injection and enforce correct branch semantics.This issue was previously flagged and remains unresolved.
github.head_refis interpolated directly in the shell, creating a template-injection risk through crafted PR branch names. Additionally, the validation script should receive the target branch (github.base_ref), not the source branch, to correctly validate release-branch version rules.🔧 Proposed fix
- name: Validate release.yaml env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BASE_REF: ${{ github.base_ref }} run: | chmod +x .github/scripts/validate-release-yaml.sh - .github/scripts/validate-release-yaml.sh "${{ github.head_ref }}" "Kuadrant" + .github/scripts/validate-release-yaml.sh "$BASE_REF" "Kuadrant"🤖 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/version-gate.yaml around lines 28 - 30, Update the workflow step invoking validate-release-yaml.sh to pass github.base_ref through an environment variable rather than interpolating github.head_ref directly in the shell. Use that environment-backed target branch as the script argument while preserving the existing Kuadrant argument and executable setup.Source: Linters/SAST tools
.github/workflows/build-images.yaml (1)
42-44: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winUnsanitized
inputs.versionenables shell injection at multiple points in the build workflow. Theworkflow_callinputinputs.versionis interpolated directly into shell code without validation; a crafted value containing shell metacharacters (e.g.$(command)) executes arbitrary code. The same unsanitised value propagates throughneeds.prepare.outputs.version_taginto downstream shell commands.
.github/workflows/build-images.yaml#L42-L44: Passinputs.versionvia an environment variable and validate it against a semver regex before use in theifcondition andechostatements..github/workflows/build-images.yaml#L157-L159: Onceinputs.versionis sanitised upstream in thepreparejob,version_tagis safe here; no separate fix needed at this site.🔒 Proposed fix for the root cause
- name: Set Authorino build info id: build-info + env: + INPUT_VERSION: ${{ inputs.version }} run: | - if [[ -n "${{ inputs.version }}" ]]; then - echo "version=${{ inputs.version }}" >> $GITHUB_OUTPUT - echo "version_tag=v${{ inputs.version }}" >> $GITHUB_OUTPUT + if [[ -n "$INPUT_VERSION" ]]; then + if ! [[ "$INPUT_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then + echo "::error::Invalid version input: $INPUT_VERSION" + exit 1 + fi + printf 'version=%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT" + printf 'version_tag=v%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT" elif [[ ${GITHUB_REF_NAME/\//-} =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then🤖 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-images.yaml around lines 42 - 44, Sanitize the version in the prepare job at the step containing the version output assignments by passing inputs.version through an environment variable, validating it against the expected semver regex, and only then using it in the condition and echo statements. At .github/workflows/build-images.yaml lines 157-159, make no direct change because downstream version_tag usage is corrected by the upstream sanitization.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/scripts/parse-version.sh (1)
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider grouping the output redirects.
Shellcheck SC2129 suggests using a single redirect block instead of individual
>>on each line. This is a minor readability improvement.♻️ Optional refactor
-echo "version=$VERSION" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "major=$MAJOR" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "minor=$MINOR" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "patch=$PATCH" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "release-branch=$RELEASE_BRANCH" >> "${GITHUB_OUTPUT:-/dev/stdout}" +{ + echo "version=$VERSION" + echo "major=$MAJOR" + echo "minor=$MINOR" + echo "patch=$PATCH" + echo "release-branch=$RELEASE_BRANCH" +} >> "${GITHUB_OUTPUT:-/dev/stdout}"🤖 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/scripts/parse-version.sh around lines 27 - 31, Refactor the output-writing block in the version parsing script to group the five output lines under a single append redirect, covering VERSION, MAJOR, MINOR, PATCH, and RELEASE_BRANCH while preserving their names and values.Source: Linters/SAST tools
.github/workflows/pre-release.yaml (1)
16-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope permissions to job level for least privilege.
contents: writeandpull-requests: writeare granted at the workflow level, but not all jobs need both. Theopen-prjob only needspull-requests: writeandcontents: read, whilesetupandprepare-releaseneedcontents: writefor pushing branches.♻️ Proposed refactor: job-level permissions
permissions: - contents: write - pull-requests: write + contents: read jobs: setup: + permissions: + contents: write runs-on: ubuntu-latest outputs: version: ${{ steps.validate.outputs.version }} release-branch: ${{ steps.validate.outputs.release-branch }} steps: ... prepare-release: + permissions: + contents: write needs: setup runs-on: ubuntu-latest steps: ... open-pr: + permissions: + pull-requests: write + contents: read needs: [setup, prepare-release] runs-on: ubuntu-latest steps: ...🤖 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/pre-release.yaml around lines 16 - 18, Move the workflow-level permissions into job-level permissions in the setup, prepare-release, and open-pr jobs. Grant setup and prepare-release only contents: write, and grant open-pr contents: read plus pull-requests: write; remove the broad top-level permissions block.
🤖 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-images.yaml:
- Line 157: Sanitize or validate the version value before it becomes the
version_tag output, ensuring only the expected Docker tag characters are
accepted and shell metacharacters cannot propagate. Update the prepare step that
derives version_tag, then use that trusted value in the docker buildx imagetools
inspect commands at both affected locations.
In @.github/workflows/pre-release.yaml:
- Around line 33-56: Update the “Validate version format” and “Create or verify
release branch” steps to pass inputs.version and inputs.source-branch through
step-level environment variables, then reference those variables in the shell
scripts instead of directly interpolating GitHub expressions. Preserve the
existing validation, release-branch derivation, checkout, and push behavior
while ensuring user input is expanded only by the shell after validation.
- Around line 76-79: Add a yq installation step to the prepare-release job
before the “Update release.yaml” step, then keep the existing yq invocation in
that step unchanged.
In @.github/workflows/version-gate.yaml:
- Line 17: Update the actions/checkout@v4 step in the workflow to set
persist-credentials to false, ensuring the validation workflow does not retain
GITHUB_TOKEN credentials after checkout.
---
Duplicate comments:
In @.github/workflows/build-images.yaml:
- Around line 42-44: Sanitize the version in the prepare job at the step
containing the version output assignments by passing inputs.version through an
environment variable, validating it against the expected semver regex, and only
then using it in the condition and echo statements. At
.github/workflows/build-images.yaml lines 157-159, make no direct change because
downstream version_tag usage is corrected by the upstream sanitization.
In @.github/workflows/version-gate.yaml:
- Around line 28-30: Update the workflow step invoking validate-release-yaml.sh
to pass github.base_ref through an environment variable rather than
interpolating github.head_ref directly in the shell. Use that environment-backed
target branch as the script argument while preserving the existing Kuadrant
argument and executable setup.
---
Nitpick comments:
In @.github/scripts/parse-version.sh:
- Around line 27-31: Refactor the output-writing block in the version parsing
script to group the five output lines under a single append redirect, covering
VERSION, MAJOR, MINOR, PATCH, and RELEASE_BRANCH while preserving their names
and values.
In @.github/workflows/pre-release.yaml:
- Around line 16-18: Move the workflow-level permissions into job-level
permissions in the setup, prepare-release, and open-pr jobs. Grant setup and
prepare-release only contents: write, and grant open-pr contents: read plus
pull-requests: write; remove the broad top-level permissions block.
🪄 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: e0cfe4f6-a9d2-4a33-a44c-371659493161
📒 Files selected for processing (8)
.github/scripts/parse-version.sh.github/scripts/validate-release-yaml.sh.github/workflows/build-images.yaml.github/workflows/pre-release.yaml.github/workflows/release.yaml.github/workflows/version-gate.yamlRELEASE.mdrelease.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- release.yaml
- .github/scripts/validate-release-yaml.sh
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.github/workflows/version-gate.yaml (1)
28-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse
github.base_refvia env to avoid injection and enforce correct branch semantics.This issue was previously flagged and remains unresolved.
github.head_refis interpolated directly in the shell, creating a template-injection risk through crafted PR branch names. Additionally, the validation script should receive the target branch (github.base_ref), not the source branch, to correctly validate release-branch version rules.🔧 Proposed fix
- name: Validate release.yaml env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BASE_REF: ${{ github.base_ref }} run: | chmod +x .github/scripts/validate-release-yaml.sh - .github/scripts/validate-release-yaml.sh "${{ github.head_ref }}" "Kuadrant" + .github/scripts/validate-release-yaml.sh "$BASE_REF" "Kuadrant"🤖 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/version-gate.yaml around lines 28 - 30, Update the workflow step invoking validate-release-yaml.sh to pass github.base_ref through an environment variable rather than interpolating github.head_ref directly in the shell. Use that environment-backed target branch as the script argument while preserving the existing Kuadrant argument and executable setup.Source: Linters/SAST tools
.github/workflows/build-images.yaml (1)
42-44: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winUnsanitized
inputs.versionenables shell injection at multiple points in the build workflow. Theworkflow_callinputinputs.versionis interpolated directly into shell code without validation; a crafted value containing shell metacharacters (e.g.$(command)) executes arbitrary code. The same unsanitised value propagates throughneeds.prepare.outputs.version_taginto downstream shell commands.
.github/workflows/build-images.yaml#L42-L44: Passinputs.versionvia an environment variable and validate it against a semver regex before use in theifcondition andechostatements..github/workflows/build-images.yaml#L157-L159: Onceinputs.versionis sanitised upstream in thepreparejob,version_tagis safe here; no separate fix needed at this site.🔒 Proposed fix for the root cause
- name: Set Authorino build info id: build-info + env: + INPUT_VERSION: ${{ inputs.version }} run: | - if [[ -n "${{ inputs.version }}" ]]; then - echo "version=${{ inputs.version }}" >> $GITHUB_OUTPUT - echo "version_tag=v${{ inputs.version }}" >> $GITHUB_OUTPUT + if [[ -n "$INPUT_VERSION" ]]; then + if ! [[ "$INPUT_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then + echo "::error::Invalid version input: $INPUT_VERSION" + exit 1 + fi + printf 'version=%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT" + printf 'version_tag=v%s\n' "$INPUT_VERSION" >> "$GITHUB_OUTPUT" elif [[ ${GITHUB_REF_NAME/\//-} =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then🤖 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-images.yaml around lines 42 - 44, Sanitize the version in the prepare job at the step containing the version output assignments by passing inputs.version through an environment variable, validating it against the expected semver regex, and only then using it in the condition and echo statements. At .github/workflows/build-images.yaml lines 157-159, make no direct change because downstream version_tag usage is corrected by the upstream sanitization.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/scripts/parse-version.sh (1)
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider grouping the output redirects.
Shellcheck SC2129 suggests using a single redirect block instead of individual
>>on each line. This is a minor readability improvement.♻️ Optional refactor
-echo "version=$VERSION" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "major=$MAJOR" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "minor=$MINOR" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "patch=$PATCH" >> "${GITHUB_OUTPUT:-/dev/stdout}" -echo "release-branch=$RELEASE_BRANCH" >> "${GITHUB_OUTPUT:-/dev/stdout}" +{ + echo "version=$VERSION" + echo "major=$MAJOR" + echo "minor=$MINOR" + echo "patch=$PATCH" + echo "release-branch=$RELEASE_BRANCH" +} >> "${GITHUB_OUTPUT:-/dev/stdout}"🤖 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/scripts/parse-version.sh around lines 27 - 31, Refactor the output-writing block in the version parsing script to group the five output lines under a single append redirect, covering VERSION, MAJOR, MINOR, PATCH, and RELEASE_BRANCH while preserving their names and values.Source: Linters/SAST tools
.github/workflows/pre-release.yaml (1)
16-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope permissions to job level for least privilege.
contents: writeandpull-requests: writeare granted at the workflow level, but not all jobs need both. Theopen-prjob only needspull-requests: writeandcontents: read, whilesetupandprepare-releaseneedcontents: writefor pushing branches.♻️ Proposed refactor: job-level permissions
permissions: - contents: write - pull-requests: write + contents: read jobs: setup: + permissions: + contents: write runs-on: ubuntu-latest outputs: version: ${{ steps.validate.outputs.version }} release-branch: ${{ steps.validate.outputs.release-branch }} steps: ... prepare-release: + permissions: + contents: write needs: setup runs-on: ubuntu-latest steps: ... open-pr: + permissions: + pull-requests: write + contents: read needs: [setup, prepare-release] runs-on: ubuntu-latest steps: ...🤖 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/pre-release.yaml around lines 16 - 18, Move the workflow-level permissions into job-level permissions in the setup, prepare-release, and open-pr jobs. Grant setup and prepare-release only contents: write, and grant open-pr contents: read plus pull-requests: write; remove the broad top-level permissions block.
🤖 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-images.yaml:
- Line 157: Sanitize or validate the version value before it becomes the
version_tag output, ensuring only the expected Docker tag characters are
accepted and shell metacharacters cannot propagate. Update the prepare step that
derives version_tag, then use that trusted value in the docker buildx imagetools
inspect commands at both affected locations.
In @.github/workflows/pre-release.yaml:
- Around line 33-56: Update the “Validate version format” and “Create or verify
release branch” steps to pass inputs.version and inputs.source-branch through
step-level environment variables, then reference those variables in the shell
scripts instead of directly interpolating GitHub expressions. Preserve the
existing validation, release-branch derivation, checkout, and push behavior
while ensuring user input is expanded only by the shell after validation.
- Around line 76-79: Add a yq installation step to the prepare-release job
before the “Update release.yaml” step, then keep the existing yq invocation in
that step unchanged.
In @.github/workflows/version-gate.yaml:
- Line 17: Update the actions/checkout@v4 step in the workflow to set
persist-credentials to false, ensuring the validation workflow does not retain
GITHUB_TOKEN credentials after checkout.
---
Duplicate comments:
In @.github/workflows/build-images.yaml:
- Around line 42-44: Sanitize the version in the prepare job at the step
containing the version output assignments by passing inputs.version through an
environment variable, validating it against the expected semver regex, and only
then using it in the condition and echo statements. At
.github/workflows/build-images.yaml lines 157-159, make no direct change because
downstream version_tag usage is corrected by the upstream sanitization.
In @.github/workflows/version-gate.yaml:
- Around line 28-30: Update the workflow step invoking validate-release-yaml.sh
to pass github.base_ref through an environment variable rather than
interpolating github.head_ref directly in the shell. Use that environment-backed
target branch as the script argument while preserving the existing Kuadrant
argument and executable setup.
---
Nitpick comments:
In @.github/scripts/parse-version.sh:
- Around line 27-31: Refactor the output-writing block in the version parsing
script to group the five output lines under a single append redirect, covering
VERSION, MAJOR, MINOR, PATCH, and RELEASE_BRANCH while preserving their names
and values.
In @.github/workflows/pre-release.yaml:
- Around line 16-18: Move the workflow-level permissions into job-level
permissions in the setup, prepare-release, and open-pr jobs. Grant setup and
prepare-release only contents: write, and grant open-pr contents: read plus
pull-requests: write; remove the broad top-level permissions block.
🪄 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: e0cfe4f6-a9d2-4a33-a44c-371659493161
📒 Files selected for processing (8)
.github/scripts/parse-version.sh.github/scripts/validate-release-yaml.sh.github/workflows/build-images.yaml.github/workflows/pre-release.yaml.github/workflows/release.yaml.github/workflows/version-gate.yamlRELEASE.mdrelease.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- release.yaml
- .github/scripts/validate-release-yaml.sh
🛑 Comments failed to post (4)
.github/workflows/build-images.yaml (1)
157-157: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Downstream shell injection via
needs.prepare.outputs.version_tag.
version_tagis derived from the unsanitizedinputs.version(lines 43–44) and interpolated directly intodocker buildx imagetools inspectcommands at lines 157 and 159. Ifinputs.versioncontains shell metacharacters,version_tagcarries them forward and they execute here. This shares the same root cause as the template injection at lines 42–44.Also applies to: 159-159
🧰 Tools
🪛 zizmor (1.26.1)
[info] 157-157: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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-images.yaml at line 157, Sanitize or validate the version value before it becomes the version_tag output, ensuring only the expected Docker tag characters are accepted and shell metacharacters cannot propagate. Update the prepare step that derives version_tag, then use that trusted value in the docker buildx imagetools inspect commands at both affected locations.Source: Linters/SAST tools
.github/workflows/pre-release.yaml (2)
33-56: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pass user inputs through environment variables to prevent shell injection.
inputs.version(line 36) andinputs.source-branch(line 53) are directly interpolated intorunscripts. GitHub Actions expands${{ }}expressions before the shell executes, so a crafted input containing shell metacharacters would be executed before the regex validation on line 37 can reject it. Map both inputs to environment variables and reference them with native shell syntax.🛡️ Proposed fix: use env variables for user inputs
- name: Validate version format id: validate + env: + INPUT_VERSION: ${{ inputs.version }} run: | - VERSION="${{ inputs.version }}" + VERSION="$INPUT_VERSION" if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then echo "::error::Invalid semver version: $VERSION" exit 1 fi MAJOR=$(echo "$VERSION" | cut -d. -f1) MINOR=$(echo "$VERSION" | cut -d. -f2) RELEASE_BRANCH="release-${MAJOR}.${MINOR}" echo "version=$VERSION" >> "$GITHUB_OUTPUT" echo "release-branch=$RELEASE_BRANCH" >> "$GITHUB_OUTPUT" - name: Create or verify release branch + env: + SOURCE_BRANCH: ${{ inputs.source-branch }} run: | RELEASE_BRANCH="${{ steps.validate.outputs.release-branch }}" if git ls-remote --exit-code origin "refs/heads/${RELEASE_BRANCH}" >/dev/null 2>&1; then echo "Release branch '${RELEASE_BRANCH}' already exists" else - echo "Creating release branch '${RELEASE_BRANCH}' from '${{ inputs.source-branch }}'" + echo "Creating release branch '${RELEASE_BRANCH}' from '$SOURCE_BRANCH'" git checkout -b "${RELEASE_BRANCH}" git push origin "${RELEASE_BRANCH}" fi📝 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.- name: Validate version format id: validate env: INPUT_VERSION: ${{ inputs.version }} run: | VERSION="$INPUT_VERSION" if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then echo "::error::Invalid semver version: $VERSION" exit 1 fi MAJOR=$(echo "$VERSION" | cut -d. -f1) MINOR=$(echo "$VERSION" | cut -d. -f2) RELEASE_BRANCH="release-${MAJOR}.${MINOR}" echo "version=$VERSION" >> "$GITHUB_OUTPUT" echo "release-branch=$RELEASE_BRANCH" >> "$GITHUB_OUTPUT" - name: Create or verify release branch env: SOURCE_BRANCH: ${{ inputs.source-branch }} run: | RELEASE_BRANCH="${{ steps.validate.outputs.release-branch }}" if git ls-remote --exit-code origin "refs/heads/${RELEASE_BRANCH}" >/dev/null 2>&1; then echo "Release branch '${RELEASE_BRANCH}' already exists" else echo "Creating release branch '${RELEASE_BRANCH}' from '$SOURCE_BRANCH'" git checkout -b "${RELEASE_BRANCH}" git push origin "${RELEASE_BRANCH}" fi🧰 Tools
🪛 zizmor (1.26.1)
[error] 36-36: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 49-49: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 53-53: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/pre-release.yaml around lines 33 - 56, Update the “Validate version format” and “Create or verify release branch” steps to pass inputs.version and inputs.source-branch through step-level environment variables, then reference those variables in the shell scripts instead of directly interpolating GitHub expressions. Preserve the existing validation, release-branch derivation, checkout, and push behavior while ensuring user input is expanded only by the shell after validation.
76-79: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Missing
yqinstallation inprepare-releasejob.The
Update release.yamlstep usesyqon line 79, butyqis never installed in this job. Bothrelease.yaml(lines 26–30) andversion-gate.yamlexplicitly installyq, confirming it is not pre-installed on the runner. The workflow will fail here every time.🐛 Proposed fix: add yq installation step
- name: Create pre-release branch run: | PRE_RELEASE_BRANCH="pre-release-v${{ needs.setup.outputs.version }}" if git ls-remote --exit-code origin "refs/heads/${PRE_RELEASE_BRANCH}" >/dev/null 2>&1; then echo "::error::Pre-release branch '${PRE_RELEASE_BRANCH}' already exists. Delete it first or use a different version." exit 1 fi git checkout -b "${PRE_RELEASE_BRANCH}" + - name: Install yq + run: | + sudo wget -qO /usr/local/bin/yq \ + https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 + sudo chmod +x /usr/local/bin/yq + - name: Update release.yaml run: | VERSION="${{ needs.setup.outputs.version }}" yq -i ".authorino.version = \"${VERSION}\"" release.yaml📝 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.- name: Install yq run: | sudo wget -qO /usr/local/bin/yq \ https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 sudo chmod +x /usr/local/bin/yq - name: Update release.yaml run: | VERSION="${{ needs.setup.outputs.version }}" yq -i ".authorino.version = \"${VERSION}\"" release.yaml🧰 Tools
🪛 zizmor (1.26.1)
[info] 78-78: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/pre-release.yaml around lines 76 - 79, Add a yq installation step to the prepare-release job before the “Update release.yaml” step, then keep the existing yq invocation in that step unchanged..github/workflows/version-gate.yaml (1)
17-17: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Set
persist-credentials: falseon the checkout step.
actions/checkoutpersists theGITHUB_TOKENin.git/configby default. This workflow only reads files for validation and does not need git credentials afterward. Disabling credential persistence reduces the attack surface if a subsequent step is compromised.🔧 Proposed fix
- uses: actions/checkout@v4 + with: + persist-credentials: false📝 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.- uses: actions/checkout@v4 with: persist-credentials: false🧰 Tools
🪛 GitHub Check: Check pinned actions
[failure] 17-17: Ratchet - Unpinned Reference
.github/workflows/version-gate.yaml:17:15: The referenceactions/checkout@v4is unpinned. Either pin the reference to a SHA or mark the line withratchet:exclude.🪛 zizmor (1.26.1)
[warning] 17-17: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/version-gate.yaml at line 17, Update the actions/checkout@v4 step in the workflow to set persist-credentials to false, ensuring the validation workflow does not retain GITHUB_TOKEN credentials after checkout.Source: Linters/SAST tools
Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
UPDATE: Ratchet GitHub actions pinning
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pre-release.yaml (1)
33-56: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPrevent shell-template injection via workflow inputs.
Directly interpolating
${{ inputs.version }}and${{ inputs.source-branch }}into the bash script allows arbitrary command execution if these inputs contain shell metacharacters. Map these inputs to environment variables and reference them using standard shell syntax.🛠 Proposed fix
- name: Validate version format id: validate + env: + INPUT_VERSION: ${{ inputs.version }} run: | - VERSION="${{ inputs.version }}" + VERSION="$INPUT_VERSION" if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then echo "::error::Invalid semver version: $VERSION" exit 1 fi MAJOR=$(echo "$VERSION" | cut -d. -f1) MINOR=$(echo "$VERSION" | cut -d. -f2) RELEASE_BRANCH="release-${MAJOR}.${MINOR}" echo "version=$VERSION" >> "$GITHUB_OUTPUT" echo "release-branch=$RELEASE_BRANCH" >> "$GITHUB_OUTPUT" - name: Create or verify release branch + env: + INPUT_SOURCE_BRANCH: ${{ inputs.source-branch }} run: | RELEASE_BRANCH="${{ steps.validate.outputs.release-branch }}" if git ls-remote --exit-code origin "refs/heads/${RELEASE_BRANCH}" >/dev/null 2>&1; then echo "Release branch '${RELEASE_BRANCH}' already exists" else - echo "Creating release branch '${RELEASE_BRANCH}' from '${{ inputs.source-branch }}'" + echo "Creating release branch '${RELEASE_BRANCH}' from '$INPUT_SOURCE_BRANCH'" git checkout -b "${RELEASE_BRANCH}" git push origin "${RELEASE_BRANCH}" fi🤖 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/pre-release.yaml around lines 33 - 56, Prevent shell injection in the “Validate version format” and “Create or verify release branch” steps by mapping inputs.version and inputs.source-branch through the steps’ environment variables, then referencing those variables with standard shell syntax inside bash. Preserve the existing semver validation, release-branch derivation, and branch creation behavior while removing direct ${{ inputs.* }} interpolation from run scripts.
♻️ Duplicate comments (3)
.github/workflows/version-gate.yaml (1)
25-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse
github.base_refvia an environment variable to prevent injection and target the correct branch.The validation script should evaluate the target release branch (
github.base_ref) rather than the source branch (github.head_ref). In addition, directly interpolating PR branch names into the shell script poses a command injection risk. Map the target branch to an environment variable in theenvblock.🛠 Proposed fix
- name: Validate release.yaml env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BASE_REF: ${{ github.base_ref }} run: | chmod +x .github/scripts/validate-release-yaml.sh - .github/scripts/validate-release-yaml.sh "${{ github.head_ref }}" "Kuadrant" + .github/scripts/validate-release-yaml.sh "$BASE_REF" "Kuadrant"🤖 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/version-gate.yaml around lines 25 - 30, Update the “Validate release.yaml” workflow step to map github.base_ref to an environment variable in its env block, then pass that variable to validate-release-yaml.sh instead of directly interpolating github.head_ref. Preserve the existing GH_TOKEN configuration and validation command behavior..github/workflows/release.yaml (2)
38-44: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPass
inputs.release-branchvia an environment variable to prevent shell injection.Directly interpolating the workflow input
inputs.release-branchinto the bash script allows shell command injection. Map it to an environment variable in theenvblock and use native shell syntax.🛠 Proposed fix
- name: Validate branch matches version + env: + INPUT_RELEASE_BRANCH: ${{ inputs.release-branch }} run: | EXPECTED_BRANCH="release-${{ steps.parse.outputs.major }}.${{ steps.parse.outputs.minor }}" - if [[ "${{ inputs.release-branch }}" != "$EXPECTED_BRANCH" ]]; then - echo "::error::Branch '${{ inputs.release-branch }}' does not match version ${{ steps.parse.outputs.version }} (expected branch: $EXPECTED_BRANCH)" + if [[ "$INPUT_RELEASE_BRANCH" != "$EXPECTED_BRANCH" ]]; then + echo "::error::Branch '$INPUT_RELEASE_BRANCH' does not match version ${{ steps.parse.outputs.version }} (expected branch: $EXPECTED_BRANCH)" exit 1 fi🤖 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 38 - 44, Update the “Validate branch matches version” step to pass inputs.release-branch through the step’s env block, then reference that environment variable with native Bash syntax in the comparison and error message instead of interpolating the workflow expression directly.
87-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake tag creation idempotent for safe reruns.
If a previous run failed after pushing the tag, subsequent reruns will unconditionally fail here, preventing recovery. Verify if the tag already points to the current HEAD instead of just checking for its existence.
🛠 Proposed fix
- name: Create and push tag run: | VERSION="${{ needs.read-version.outputs.version }}" TAG="v${VERSION}" if git rev-parse "$TAG" >/dev/null 2>&1; then - echo "::error::Tag $TAG already exists" - exit 1 + TAG_SHA="$(git rev-list -n1 "$TAG")" + HEAD_SHA="$(git rev-parse HEAD)" + if [[ "$TAG_SHA" != "$HEAD_SHA" ]]; then + echo "::error::Tag $TAG already exists and points to $TAG_SHA, not $HEAD_SHA" + exit 1 + fi + echo "Tag $TAG already exists at HEAD; continuing" + exit 0 fi git config user.name "github-actions[bot]" git config user.email "github-actions[bot]`@users.noreply.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/release.yaml around lines 87 - 94, Update the “Create and push tag” step to compare an existing $TAG’s commit with the current HEAD: continue successfully when they match, but report an error and exit when the tag points elsewhere. Preserve normal tag creation and pushing when $TAG does not exist.
🧹 Nitpick comments (3)
.github/workflows/release.yaml (2)
22-24: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueDisable credential persistence on read-only checkouts.
These workflow jobs only read the repository or use a dedicated token for API calls; they do not push any commits back to Git. The
actions/checkoutaction defaults to persisting theGITHUB_TOKENin the local git config, which presents an unnecessary security risk here. Addpersist-credentials: falseto all of these checkout steps.
.github/workflows/release.yaml#L22-L24: addpersist-credentials: falseto theread-versionjob checkout..github/workflows/release.yaml#L60-L62: addpersist-credentials: falseto thesmoke-testsjob checkout..github/workflows/release.yaml#L112-L115: addpersist-credentials: falseto thecreate-releasejob checkout..github/workflows/version-gate.yaml#L17-L17: add awith:block settingpersist-credentials: falseto thevalidate-release-yamljob checkout..github/workflows/pre-release.yaml#L108-L108: add awith:block settingpersist-credentials: falseto theopen-prjob checkout.🤖 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 22 - 24, Disable credential persistence for every read-only checkout by setting persist-credentials to false in the checkout steps for .github/workflows/release.yaml lines 22-24, 60-62, and 112-115, plus .github/workflows/version-gate.yaml line 17 and .github/workflows/pre-release.yaml line 108; add a with block where needed while preserving existing checkout options.
126-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a Bash array for CLI arguments to prevent word splitting.
Appending command-line arguments to a string (
NOTES_ARGS) and passing it unquoted can cause unexpected word splitting if any variables contain spaces. Utilizing a Bash array is safer and more robust.🛠 Proposed fix
- NOTES_ARGS="--generate-notes" + NOTES_ARGS=("--generate-notes") if [[ -n "$PREV_TAG" ]]; then - NOTES_ARGS="$NOTES_ARGS --notes-start-tag $PREV_TAG" + NOTES_ARGS+=("--notes-start-tag" "$PREV_TAG") fi gh release create "$TAG" \ --title "Release $TAG" \ - $NOTES_ARGS + "${NOTES_ARGS[@]}"🤖 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 126 - 133, Replace the string-based NOTES_ARGS construction with a Bash array, appending --generate-notes and conditionally --notes-start-tag plus PREV_TAG as separate elements. Pass the array to gh release create using quoted array expansion, preserving the existing arguments and behavior..github/workflows/pre-release.yaml (1)
91-102: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHandle identical branches to prevent pull request creation errors.
If there are no changes to commit (e.g.,
release.yamlalready has the target version and manifests are up to date), the pre-release branch will remain identical to the release branch. This will cause thegh pr createstep in theopen-prjob to fail, as GitHub requires at least one commit difference.
You can bypass this by allowing an empty commit to ensure the pull request can be successfully opened.🛠 Proposed fix
- name: Commit and push changes run: | VERSION="${{ needs.setup.outputs.version }}" git config user.name "github-actions[bot]" git config user.email "github-actions[bot]`@users.noreply.github.com`" git add -A - if git diff --cached --quiet; then - echo "No changes to commit" - else - git commit -m "chore: prepare release v${VERSION}" - fi + + # Commit the changes or create an empty commit to ensure the branch differs from the base, + # which is required to successfully open a pull request. + git commit --allow-empty -m "chore: prepare release v${VERSION}" git push origin "pre-release-v${VERSION}"🤖 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/pre-release.yaml around lines 91 - 102, Update the “Commit and push changes” step to create an empty commit when git diff --cached --quiet reports no staged changes, instead of only printing “No changes to commit.” Preserve the existing release commit message and push behavior so the pre-release branch always has a commit difference for the open-pr job.
🤖 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.
Outside diff comments:
In @.github/workflows/pre-release.yaml:
- Around line 33-56: Prevent shell injection in the “Validate version format”
and “Create or verify release branch” steps by mapping inputs.version and
inputs.source-branch through the steps’ environment variables, then referencing
those variables with standard shell syntax inside bash. Preserve the existing
semver validation, release-branch derivation, and branch creation behavior while
removing direct ${{ inputs.* }} interpolation from run scripts.
---
Duplicate comments:
In @.github/workflows/release.yaml:
- Around line 38-44: Update the “Validate branch matches version” step to pass
inputs.release-branch through the step’s env block, then reference that
environment variable with native Bash syntax in the comparison and error message
instead of interpolating the workflow expression directly.
- Around line 87-94: Update the “Create and push tag” step to compare an
existing $TAG’s commit with the current HEAD: continue successfully when they
match, but report an error and exit when the tag points elsewhere. Preserve
normal tag creation and pushing when $TAG does not exist.
In @.github/workflows/version-gate.yaml:
- Around line 25-30: Update the “Validate release.yaml” workflow step to map
github.base_ref to an environment variable in its env block, then pass that
variable to validate-release-yaml.sh instead of directly interpolating
github.head_ref. Preserve the existing GH_TOKEN configuration and validation
command behavior.
---
Nitpick comments:
In @.github/workflows/pre-release.yaml:
- Around line 91-102: Update the “Commit and push changes” step to create an
empty commit when git diff --cached --quiet reports no staged changes, instead
of only printing “No changes to commit.” Preserve the existing release commit
message and push behavior so the pre-release branch always has a commit
difference for the open-pr job.
In @.github/workflows/release.yaml:
- Around line 22-24: Disable credential persistence for every read-only checkout
by setting persist-credentials to false in the checkout steps for
.github/workflows/release.yaml lines 22-24, 60-62, and 112-115, plus
.github/workflows/version-gate.yaml line 17 and
.github/workflows/pre-release.yaml line 108; add a with block where needed while
preserving existing checkout options.
- Around line 126-133: Replace the string-based NOTES_ARGS construction with a
Bash array, appending --generate-notes and conditionally --notes-start-tag plus
PREV_TAG as separate elements. Pass the array to gh release create using quoted
array expansion, preserving the existing arguments and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a20bc8fd-7944-479b-91bb-baac39704eee
📒 Files selected for processing (3)
.github/workflows/pre-release.yaml.github/workflows/release.yaml.github/workflows/version-gate.yaml
guicassolato
left a comment
guicassolato
left a comment
There was a problem hiding this comment.
Process for Authorino releases today is pretty simple – though arguably requires attention due being significantly manual.
With automation, it will probably become easier, though less simple – more steps and more elements to handle (e.g. a release.yaml file) – and more abstract.
I wonder if we shouldn't include a "rollback guidance" section to RELEASE.md.
| ## Release v${VERSION} | ||
|
|
||
| This PR prepares the release of v${VERSION}. | ||
|
|
||
| ### Pre-release changes | ||
| - Updated \`release.yaml\` version to \`${VERSION}\` | ||
| - Ran \`make generate\` and \`make manifests\` | ||
|
|
||
| ### Checklist | ||
| - [ ] Version numbers are correct | ||
| - [ ] All pre-release modifications look correct | ||
| - [ ] CI checks pass | ||
| - [ ] Version gate check passes | ||
|
|
||
| ### Next steps | ||
| After merging this PR, run the **Release** workflow with branch \`${RELEASE_BRANCH}\`. | ||
| EOF |
There was a problem hiding this comment.
Won't this indentation be a problem for the body of the PR? I wonder if the text won't end up as all wrapped in a code block in markdown.
There was a problem hiding this comment.
The indentation is not a problem. You can see a PR that was open with this formatting here: Boomatang#2.
| - name: Install yq | ||
| run: | | ||
| sudo wget -qO /usr/local/bin/yq \ | ||
| https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 |
There was a problem hiding this comment.
Maybe pin yq to a specific version instead of latest, to avoid possibly breaking on external updates?
There was a problem hiding this comment.
Sure I can pin it to a version. I will check the rest of the code base to see what version is defined and use the same that.
There was a problem hiding this comment.
As branch protections are now working correctly at the org level I can push to this branch any more. This change is covered in this PR #658
@guicassolato I think this would be a good idea, but I am not sure if I am understanding what you are wanting to roll back. I am going to assume it would be the reverting of the commit the is generated during the pre-release workflow. Can you explain what you intend by roll back.
I see this in a few different ways. It does become easier to release in the global context of kuadrant. We are work towards every repo having the same release workflow (pre-release pipeline, PR review, release pipeline). I am not sure if you could call it less simple. I know authorino isn't the worse of our projects for this but it still does. When a release was made other workflows would get triggered. The person doing the release has very little context into how many workflows were, or should be triggered. And knowing if any of those workflows had fail is a different story. So do think it is less simple, it's more where the complicity is has being shifted. The
I agree it is one of our simpler projects to release, but the manual steps do make it harder. And harder to automated a global release. |
|
Because of branch protections I have reopen this PR under a different branch. That PR can be seen here. #666 |
3 participants
Summary
Introduces an automated two-phase release process for Authorino, replacing the previous manual git-tag-and-build approach. The new workflow splits every release into a Pre-release phase (branch creation, version bump, code generation, PR) and a Release phase (smoke tests, tagging, image build, GitHub Release), with a mandatory human review gate between them.
This change implements the design specified in RFC: Two-Phase Release Workflow.
Motivation
The previous release process was entirely manual: a maintainer checked out a ref, created a signed tag, pushed it, manually triggered the image build, and hand-wrote release notes. This was error-prone, undocumented in automation, and lacked pre-release validation.
The two-phase approach automates the mechanical steps while preserving human oversight at the critical review point before a release is finalized.
What changed
New workflows
pre-release.yaml) — triggered manually with a target version. Creates therelease-X.Ybranch (if needed), opens apre-release-vX.Y.Zbranch that bumpsrelease.yaml, runsmake generateandmake manifests, and opens a PR against the release branch.release.yaml) — triggered manually with a release branch name. Reads the version fromrelease.yaml, validates the branch matches, runs smoke tests (lint, unit tests, CEL tests), creates an annotated tag, builds and pushes the multi-arch container image viabuild-images.yaml, and creates the GitHub Release.version-gate.yaml) — runs on PRs targetingrelease-**branches whenrelease.yamlchanges. Validates that the version is not0.0.0on non-main branches and that any declared dependencies have published releases.New scripts
parse-version.sh— extracts and validates semver components fromrelease.yaml, used by the release workflow.validate-release-yaml.sh— validatesrelease.yamlcontent and dependency versions, used by the version gate.Modified workflows
build-images.yaml) — now supportsworkflow_callwithversionandrefinputs so the release workflow can invoke it directly. Image tagging logic updated to use the version from inputs when provided.New files
release.yaml— version source-of-truth file at the repository root. Set to0.0.0onmain(active development) and updated to the target version on release branches.Updated documentation
RELEASE.md— rewritten to document the new two-phase process, including step-by-step instructions, artifact locations.Examples
Summary by CodeRabbit
Release Notes
Documentation
Chores
release.yaml, parse and gate versioning for release branches, and verify dependency release tags.release.yamlto setauthorino.versionto0.0.0.