-
Notifications
You must be signed in to change notification settings - Fork 243
fix: ensure release tag points to non-snapshot release commit #3582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,20 +11,35 @@ on: | |
| required: true | ||
|
|
||
| env: | ||
| # set the target pom to use the input directory as root | ||
| # set the target pom to use the input directory as root | ||
| MAVEN_ARGS: -V -ntp -e -f ${{ inputs.project_dir }}/pom.xml | ||
|
|
||
| jobs: | ||
| publish: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: write | ||
| steps: | ||
| - name: Checkout "${{inputs.version_branch}}" branch | ||
| uses: actions/checkout@v7 | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: "${{inputs.version_branch}}" | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Parse release version | ||
| id: vars | ||
| run: | | ||
| RAW_TAG="${{ github.event.release.tag_name }}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: printf '%s\n' '--- repository guidance ---'
find /tmp/coderabbit-repo-knowledge/operator-framework-java-operator-sdk-ba8a0318 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- workflow files ---'
sed -n '1,180p' .github/workflows/release-project-in-dir.yml
printf '%s\n' '--- release workflow ---'
sed -n '1,180p' .github/workflows/release.yml
printf '%s\n' '--- relevant diff ---'
git diff -- .github/workflows/release-project-in-dir.yml .github/workflows/release.ymlRepository: operator-framework/java-operator-sdk Length of output: 7719 🏁 Script executed: printf '%s\n' '--- repository-wide review conventions ---'
cat /tmp/coderabbit-repo-knowledge/operator-framework-java-operator-sdk-ba8a0318/conventions/repo-wide.md
printf '%s\n' '--- workflow-specific convention candidates ---'
cat /tmp/coderabbit-repo-knowledge/operator-framework-java-operator-sdk-ba8a0318/conventions/agents-md.md
printf '%s\n' '--- all direct expression interpolations in the two workflows ---'
rg -n -C 2 '\$\{\{[^}]*github\.event\.release\.tag_name|steps\.vars\.outputs|github\.repository|inputs\.' \
.github/workflows/release-project-in-dir.yml .github/workflows/release.ymlRepository: operator-framework/java-operator-sdk Length of output: 10312 🌐 Web query:
💡 Result: In GitHub Actions, using expressions like ${{ github.event.release.tag_name }} directly within shell commands (e.g., in a run block) creates a significant risk of shell command injection [1][2][3]. This vulnerability occurs because GitHub Actions evaluates and substitutes ${{ }} expressions before the shell script runs [1][4]. If an attacker can control the input—such as by creating a specifically crafted release tag—they can inject malicious shell commands that the runner will execute [1][3]. Even if the input is wrapped in quotes, an attacker may use characters like backticks, semicolons, or additional quotes to break out of the intended context [1][2]. To mitigate this risk, follow these authoritative security practices: 1. Use Intermediate Environment Variables: Never interpolate untrusted input directly into a shell command [2][5][4]. Instead, map the expression to an environment variable in the env section, and then reference that variable using the appropriate shell syntax (e.g., "$VARIABLE" in Bash) [5][6][3]. Because the environment variable is passed as data rather than being pre-processed by the expression evaluator, it cannot alter the structure of the shell command [2][3]. Example of safe usage: env: RELEASE_TAG: ${{ github.event.release.tag_name }} run: | echo "Processing release: $RELEASE_TAG" 2. Treat Contexts as Untrusted: Contexts such as github.event.release.tag_name, issue titles, PR bodies, and head references should always be treated as untrusted input [1][7][3]. 3. Use Code Scanning: Enable GitHub code scanning with CodeQL, which includes built-in rules to detect unsafe interpolation of untrusted input in workflows [2][4]. 4. Security Hardening regarding Git Tags: Be aware that Git tags are mutable references [8][9]. Referencing actions or inputs by mutable tags (e.g., Citations:
🏁 Script executed: printf '%s\n' '--- valid Git tag-name characters relevant to shell parsing ---'
for tag in 'v5.1;touch /tmp/pwned' 'v5.1$(id)' 'v5.1`id`' 'v5.1&&id' 'v5.1"'; do
if git check-ref-format "refs/tags/$tag"; then
printf 'VALID: %q\n' "$tag"
else
printf 'INVALID: %q\n' "$tag"
fi
done
printf '%s\n' '--- exact workflow line ranges ---'
nl -ba .github/workflows/release-project-in-dir.yml | sed -n '25,75p'
nl -ba .github/workflows/release.yml | sed -n '28,70p'Repository: operator-framework/java-operator-sdk Length of output: 450 Pass release tag values through 🧰 Tools🪛 zizmor (1.29.0)[error] 32-32: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
| STRIPPED="${RAW_TAG#v}" | ||
| FINAL_VERSION="${STRIPPED%-tmp}" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I belive we don't need the -tmp suffix, we can create the tag like |
||
| FINAL_TAG="v${FINAL_VERSION}" | ||
|
|
||
| echo "raw_tag=${RAW_TAG}" >> "$GITHUB_OUTPUT" | ||
| echo "final_version=${FINAL_VERSION}" >> "$GITHUB_OUTPUT" | ||
| echo "final_tag=${FINAL_TAG}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Set up Java and Maven | ||
| uses: actions/setup-java@v6 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here this version should not be changed |
||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: 17 | ||
| distribution: temurin | ||
|
|
@@ -36,11 +51,39 @@ jobs: | |
| gpg-passphrase: MAVEN_GPG_PASSPHRASE | ||
|
|
||
| - name: Change version to release version | ||
| # Assume that RELEASE_VERSION will have form like: "v1.0.1". So we cut the "v" | ||
| run: | | ||
| ./mvnw ${MAVEN_ARGS} versions:set -DnewVersion="${RELEASE_VERSION:1}" versions:commit -DprocessAllModules | ||
| ./mvnw ${MAVEN_ARGS} versions:set -DnewVersion="${{ steps.vars.outputs.final_version }}" versions:commit -DprocessAllModules | ||
|
|
||
| - name: Commit and push release version | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have these 2 steps after maven release? that way if maven release fails, and we can easily retry. |
||
| run: | | ||
| git config --local user.email "action@github.com" | ||
| git config --local user.name "GitHub Action" | ||
| if git diff --quiet; then | ||
| echo "No version changes to commit." | ||
| else | ||
| git commit -am "Release ${{ steps.vars.outputs.final_tag }}" | ||
| git push origin HEAD:${{ inputs.version_branch }} | ||
| fi | ||
|
Comment on lines
+57
to
+66
|
||
|
|
||
| - name: Re-tag commit and update GitHub Release | ||
| env: | ||
| RELEASE_VERSION: ${{ github.event.release.tag_name }} | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| GH_REPO: ${{ github.repository }} | ||
| run: | | ||
| RAW_TAG="${{ steps.vars.outputs.raw_tag }}" | ||
| FINAL_TAG="${{ steps.vars.outputs.final_tag }}" | ||
|
|
||
| if [ "$RAW_TAG" != "$FINAL_TAG" ]; then | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't have the -temp suffix, we don't have to check this, we simply override the tag as mentioned before, then we don;t have to edit the release, since release just point to a ref name on GitHub |
||
| echo "Creating final release tag: ${FINAL_TAG}" | ||
| git tag -a "${FINAL_TAG}" -m "Release ${FINAL_TAG}" | ||
| git push origin "${FINAL_TAG}" | ||
|
|
||
| echo "Pointing GitHub release to final tag ${FINAL_TAG}" | ||
| gh release edit "${RAW_TAG}" --tag "${FINAL_TAG}" | ||
|
|
||
| echo "Deleting temporary tag ${RAW_TAG}" | ||
| git push origin --delete "${RAW_TAG}" || true | ||
|
Comment on lines
+78
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Finalize the tag after Maven Central publishing succeeds. This step creates the final tag, retargets the GitHub Release, and deletes the temporary tag before Move final-tag and release finalization after a successful deploy. Alternatively, make this sequence idempotent and verify that an existing final tag targets the intended release commit. 🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Do not hide temporary-tag deletion errors. If the remote rejects deletion, such as for a protected tag, 🤖 Prompt for AI Agents |
||
| fi | ||
|
|
||
| - name: Publish to Apache Maven Central | ||
| run: ./mvnw package deploy -Prelease | ||
|
|
@@ -49,19 +92,20 @@ jobs: | |
| MAVEN_CENTRAL_TOKEN: ${{ secrets.NEXUS_PASSWORD }} | ||
| MAVEN_GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }} | ||
|
|
||
| # This is separate job because there were issues with git after release step, was not able to commit changes. | ||
| update-working-version: | ||
| runs-on: ubuntu-latest | ||
| needs: publish | ||
| if: "!contains(github.event.release.tag_name, 'RC')" # not sure we should keep this the RC part | ||
| permissions: | ||
| contents: write | ||
| if: "!contains(github.event.release.tag_name, 'RC')" | ||
| steps: | ||
| - name: Checkout "${{inputs.version_branch}}" branch | ||
| uses: actions/checkout@v7 | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: "${{inputs.version_branch}}" | ||
|
|
||
| - name: Set up Java and Maven | ||
| uses: actions/setup-java@v6 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: 17 | ||
| distribution: temurin | ||
|
|
@@ -73,11 +117,9 @@ jobs: | |
| git config --local user.email "action@github.com" | ||
| git config --local user.name "GitHub Action" | ||
| git commit -m "Set new SNAPSHOT version into pom files." -a | ||
| env: | ||
| RELEASE_VERSION: ${{ github.event.release.tag_name }} | ||
|
|
||
| - name: Push changes to branch | ||
| uses: ad-m/github-push-action@master | ||
| with: | ||
| branch: "${{inputs.version_branch}}" | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,10 @@ jobs: | |
| runs-on: ubuntu-latest | ||
| env: | ||
| tmp_version_branch: '' | ||
| RAW_TAG: ${{ github.event.release.tag_name }} | ||
| outputs: | ||
| version_branch: ${{ steps.set-version-branch.outputs.version_branch }} | ||
| release_tag: ${{ steps.set-release-tag.outputs.release_tag }} | ||
| steps: | ||
| - if: ${{ startsWith(github.event.release.tag_name, 'v1.' ) }} | ||
| run: | | ||
|
|
@@ -31,8 +33,9 @@ jobs: | |
| echo "tmp_version_branch=v4" >> "$GITHUB_ENV" | ||
| - if: ${{ startsWith(github.event.release.tag_name, 'v5.' ) }} | ||
| run: | | ||
| RELEASE_VERSION="${{ github.event.release.tag_name }}" | ||
| RELEASE_VERSION="${RELEASE_VERSION#v}" | ||
| # Strip 'v' prefix and optional '-tmp' suffix | ||
| RELEASE_VERSION="${RAW_TAG#v}" | ||
| RELEASE_VERSION="${RELEASE_VERSION%-tmp}" | ||
| RELEASE_MAJOR_MINOR=$(echo "$RELEASE_VERSION" | cut -d. -f1-2) | ||
|
|
||
| MAIN_POM_VERSION=$(curl -fsSL "https://raw.githubusercontent.com/${{ github.repository }}/main/pom.xml" | yq -p xml '.project.version') | ||
|
|
@@ -55,16 +58,22 @@ jobs: | |
| - if: ${{ env.tmp_version_branch == '' }} | ||
| name: Fail if version_branch is not set | ||
| run: | | ||
| echo "Failed to find appropriate branch to release ${{github.event.release.tag_name}} from" | ||
| echo "Failed to find appropriate branch to release $RAW_TAG from" | ||
| exit 1 | ||
| - id: set-version-branch | ||
| name: Set version_branch if matched | ||
| run: echo "version_branch=${{env.tmp_version_branch}}" >> $GITHUB_OUTPUT | ||
| run: echo "version_branch=${{ env.tmp_version_branch }}" >> "$GITHUB_OUTPUT" | ||
| - id: set-release-tag | ||
| name: Pass release tag | ||
| run: echo "release_tag=${RAW_TAG}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| release-sdk: | ||
| needs: prepare-release | ||
| permissions: | ||
| contents: write | ||
| uses: ./.github/workflows/release-project-in-dir.yml | ||
| secrets: inherit | ||
| with: | ||
| version_branch: ${{needs.prepare-release.outputs.version_branch}} | ||
| project_dir: '.' | ||
| version_branch: ${{ needs.prepare-release.outputs.version_branch }} | ||
| release_tag: ${{ needs.prepare-release.outputs.release_tag }} | ||
|
Lavanya-N24 marked this conversation as resolved.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't declare and use this version in the sub-flow, can we adjust this for that? |
||
| project_dir: '.' | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is a mistake, v7 should be the version we want to use