Normalize WordPress.org deploy version from release tag#1791
Conversation
Strip a leading or trailing 'v' from release tag_name before passing VERSION to the WordPress.org deploy action. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughThe deploy workflow gains a new ChangesRelease Version Normalization
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)
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: 3
🤖 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/deploy-on-release-to-dot-org.yml:
- Line 66: The GitHub Action `10up/action-wordpress-plugin-deploy` is pinned to
the movable tag `@stable` which can change over time and introduces supply-chain
risk. Replace the `@stable` tag with a full commit SHA to pin the action to a
specific immutable version. You can find the appropriate commit SHA by checking
the action's repository releases or tags, then update the uses directive to
reference that SHA instead of the `@stable` tag.
- Around line 60-63: Add a validation check immediately after the
NORMALIZED_VERSION variable is set (after the line that echoes the version to
GITHUB_OUTPUT) to verify that NORMALIZED_VERSION is not empty. If
NORMALIZED_VERSION is empty, the script should exit with a clear error message
indicating that the tag format is invalid, preventing the deploy job from
proceeding with an empty version string. This guard should reference the
RAW_VERSION variable in the error message to help identify what tag was used.
- Around line 56-58: Remove the direct `${{ github.event.release.tag_name }}`
interpolation from the shell code in the `run` step to prevent command injection
vulnerabilities. Instead, define the RAW_VERSION value in the `env:` section of
the step using the GitHub context, then reference it in the shell code using the
standard shell variable syntax `$RAW_VERSION`. This ensures the tag content is
safely passed as an environment variable rather than being interpolated as raw
shell code where special characters could be exploited.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1965c96c-d224-41a6-a582-d6f179f6dab8
📒 Files selected for processing (1)
.github/workflows/deploy-on-release-to-dot-org.yml
| RAW_VERSION="${{ github.event.release.tag_name }}" | ||
| if [ -z "$RAW_VERSION" ]; then | ||
| RAW_VERSION="${GITHUB_REF_NAME}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Avoid direct ${{ }} interpolation inside run shell code (command-injection risk).
On Line 56, tag content is injected into shell source before execution. A crafted tag containing shell substitution syntax can execute commands in the runner context. Pass the value via env: and read it from a shell variable instead.
Suggested fix
- name: Normalize release version
id: release-version
+ env:
+ RELEASE_TAG_NAME: ${{ github.event.release.tag_name }}
run: |
- RAW_VERSION="${{ github.event.release.tag_name }}"
+ RAW_VERSION="$RELEASE_TAG_NAME"
if [ -z "$RAW_VERSION" ]; then
RAW_VERSION="${GITHUB_REF_NAME}"
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.
| RAW_VERSION="${{ github.event.release.tag_name }}" | |
| if [ -z "$RAW_VERSION" ]; then | |
| RAW_VERSION="${GITHUB_REF_NAME}" | |
| - name: Normalize release version | |
| id: release-version | |
| env: | |
| RELEASE_TAG_NAME: ${{ github.event.release.tag_name }} | |
| run: | | |
| RAW_VERSION="$RELEASE_TAG_NAME" | |
| if [ -z "$RAW_VERSION" ]; then | |
| RAW_VERSION="${GITHUB_REF_NAME}" | |
| fi |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 56-56: 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/deploy-on-release-to-dot-org.yml around lines 56 - 58,
Remove the direct `${{ github.event.release.tag_name }}` interpolation from the
shell code in the `run` step to prevent command injection vulnerabilities.
Instead, define the RAW_VERSION value in the `env:` section of the step using
the GitHub context, then reference it in the shell code using the standard shell
variable syntax `$RAW_VERSION`. This ensures the tag content is safely passed as
an environment variable rather than being interpolated as raw shell code where
special characters could be exploited.
Source: Linters/SAST tools
| NORMALIZED_VERSION="${RAW_VERSION#v}" | ||
| NORMALIZED_VERSION="${NORMALIZED_VERSION%v}" | ||
| echo "version=$NORMALIZED_VERSION" >> "$GITHUB_OUTPUT" | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail fast when normalized version is empty to prevent invalid deploy tags.
If the tag is v (or empty), normalization yields an empty VERSION, and the deploy step still runs. Add a guard to stop the job with a clear error before publishing.
Suggested fix
NORMALIZED_VERSION="${RAW_VERSION#v}"
NORMALIZED_VERSION="${NORMALIZED_VERSION%v}"
+ if [ -z "$NORMALIZED_VERSION" ]; then
+ echo "Normalized version is empty. Check release tag format." >&2
+ exit 1
+ fi
echo "version=$NORMALIZED_VERSION" >> "$GITHUB_OUTPUT"🤖 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/deploy-on-release-to-dot-org.yml around lines 60 - 63, Add
a validation check immediately after the NORMALIZED_VERSION variable is set
(after the line that echoes the version to GITHUB_OUTPUT) to verify that
NORMALIZED_VERSION is not empty. If NORMALIZED_VERSION is empty, the script
should exit with a clear error message indicating that the tag format is
invalid, preventing the deploy job from proceeding with an empty version string.
This guard should reference the RAW_VERSION variable in the error message to
help identify what tag was used.
|
|
||
| - name: WordPress plugin deploy | ||
| id: deploy | ||
| uses: 10up/action-wordpress-plugin-deploy@stable |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin third-party GitHub Action to a full commit SHA.
Line 66 uses a movable tag (@stable). Pinning to an immutable SHA is required to prevent supply-chain drift in release workflows.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 66-66: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/deploy-on-release-to-dot-org.yml at line 66, The GitHub
Action `10up/action-wordpress-plugin-deploy` is pinned to the movable tag
`@stable` which can change over time and introduces supply-chain risk. Replace
the `@stable` tag with a full commit SHA to pin the action to a specific
immutable version. You can find the appropriate commit SHA by checking the
action's repository releases or tags, then update the uses directive to
reference that SHA instead of the `@stable` tag.
Source: Linters/SAST tools
|
Closing as superseded: develop already contains d9e4de8 ("fix: strip leading v from release tag before using as SVN version"), which landed after this branch was cut and does the same core normalization — that competing edit at the same spot in the workflow is also why this PR shows a merge conflict. The two extras here don't survive scrutiny:
If we want to harden the manual-dispatch path, that should be a small separate PR that derives the version from the plugin header (or fails loudly) rather than falling back to the ref name. |
1 participant
Summary
Why
A release tag containing a v prefix/suffix produced an incorrect WordPress.org SVN tag.
Summary by CodeRabbit