Skip to content

ci: route github_release version parsing through env, not shell interpolation - #12230

Open
camgrimsec wants to merge 1 commit into
deepset-ai:mainfrom
camgrimsec:ci-github-release-version-parse-env
Open

ci: route github_release version parsing through env, not shell interpolation#12230
camgrimsec wants to merge 1 commit into
deepset-ai:mainfrom
camgrimsec:ci-github-release-version-parse-env

Conversation

@camgrimsec

Copy link
Copy Markdown
Contributor

Proposed change

Follow-up in the same style as #11856 and #11857: move the one remaining ${{ ... }} expansion inside a run: body in github_release.yml into the step's env: block.

Before

- name: Delete next version rc0 tag in the CI environment
  run: |
    # Parse version X.Y.Z and increment Y for next minor version
    IFS='.' read -r MAJOR MINOR _ <<< "${{ steps.version.outputs.current_release }}"
    NEXT_MINOR=$((MINOR + 1))
    NEXT_TAG="v${MAJOR}.${NEXT_MINOR}.0-rc0"
    ...

After

- name: Delete next version rc0 tag in the CI environment
  env:
    CURRENT_RELEASE: ${{ steps.version.outputs.current_release }}
  run: |
    # Parse version X.Y.Z and increment Y for next minor version
    IFS='.' read -r MAJOR MINOR _ <<< "$CURRENT_RELEASE"
    NEXT_MINOR=$((MINOR + 1))
    NEXT_TAG="v${MAJOR}.${NEXT_MINOR}.0-rc0"
    ...

Two lines added (an env: header plus CURRENT_RELEASE:), one line changed ("$CURRENT_RELEASE" in place of the ${{ }} inside the here-string). One file touched.

Why

The value read here (steps.version.outputs.current_release) is produced by awk -F \\- '{print $1}' < VERSION.txt, so today the here-string is safe as long as VERSION.txt on the checked-out ref is well-formed. That's why this is a low-risk, mechanical change: it's the same pattern already used everywhere else in the release workflow after #11856 and #11857.

The reason to keep extending it here:

  1. This step runs with contents: write (the job scope for ncipollo/release-action further down) and later invokes git tag -d "$NEXT_TAG". If anything ever makes it into VERSION.txt other than awk's first - field (a manual edit, a partial commit on a release branch, a mis-parsed prerelease format), an unexpected token would land directly in the shell body.
  2. GitHub's security-hardening guide for GitHub Actions calls out this exact pattern (run: body reading a ${{ steps.*.outputs.* }} value) and recommends passing it through env:.
  3. It closes the last template-injection hit for github_release.yml; every other ${{ ... }} in the file is already either in an env: block or an if: / with: field.

What is not changed

  • The awk -F \\- '{print $1}' < VERSION.txt parsing step above.
  • The IFS='.' read -r MAJOR MINOR _ split and the NEXT_MINOR = MINOR + 1 arithmetic.
  • The NEXT_TAG format string.
  • The subsequent git rev-parse --verify / git tag -d calls.
  • The neighboring Generate release notes step, which already binds EARLIEST_VERSION via env:.

The tag that gets deleted, the branch it's deleted from, and the condition under which the step runs are all identical to the previous behavior.

Verification

  • python3 -c "import yaml; yaml.safe_load(open('.github/workflows/github_release.yml'))" succeeds.
  • Diff is 4 lines total (+3, -1) in one file.
  • Every ${{ ... }} expression that previously appeared inside a run: body in this file is now bound at the step's env:.

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings — N/A, this is a CI-only change with no runtime impact
  • I've used one of the conventional commit types for my PR title
  • I documented my code
  • I ran pre-commit hooks and fixed any issue — N/A, YAML file, no code hooks apply

Thanks.

@camgrimsec
camgrimsec requested a review from a team as a code owner August 4, 2026 00:25
@camgrimsec
camgrimsec requested review from davidsbatista and removed request for a team August 4, 2026 00:25
@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

@camgrimsec is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Hi @camgrimsec, thanks for your interest in contributing to Haystack! 🙏

⚠️ You currently have 4 open pull requests in this repository (#12229, #11742, #11737 and this one). Our review capacity is limited, so please hold off opening more PRs until we've had a chance to review your first 2 open PRs. This helps us give each contribution the attention it deserves. Thank you!

This is an automated message to help us keep the review queue healthy.

…polation

The Delete next version rc0 tag in the CI environment step parsed
${{ steps.version.outputs.current_release }} by interpolating it
directly into a bash here-string (IFS='.' read <<< "..."). The value
originates from VERSION.txt in the checked-out ref, so a poisoned
VERSION.txt on a fetched branch could inject a shell token into a step
that later runs git tag -d in a job with contents: write.

Route the value through the step env: block and reference it as
"$CURRENT_RELEASE" so the shell treats it as an opaque string.

Behavior is unchanged: same IFS split, same NEXT_MINOR arithmetic,
same NEXT_TAG format. Same env-var pattern used in the release
workflow (deepset-ai#11856, deepset-ai#11857).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant