Skip to content

ci: pass the PR description through env instead of interpolating it into the shell - #3319

Open
kobihikri wants to merge 1 commit into
FuelLabs:masterfrom
kobihikri:ci/pr-body-via-env
Open

kobihikri wants to merge 1 commit into
FuelLabs:masterfrom
kobihikri:ci/pr-body-via-env

Conversation

@kobihikri

Copy link
Copy Markdown

Hi, and thanks for fuel-core.

In .github/workflows/create_version.yml, the PR description is pasted straight into a shell command:

version=$(echo "${{ github.event.pull_request.body }}" | grep -oP '## Version \K[0-9]+\.[0-9]+\.[0-9]+')

Actions expands ${{ ... }} before the shell ever sees the line, so the PR body becomes part of the script text rather than data passed to echo. Inside double quotes, $(...) and backticks still run, so a PR body containing something like $(id > /tmp/x) executes on the runner.

It is not wide open, and I want to be accurate about that: the job only runs behind if: github.event.label.name == 'pr release', so someone with write access has to apply the pr release label first, and on a fork PR the GITHUB_TOKEN is read-only regardless of the contents: write request. What is left is that the label is the only thing standing between an arbitrary PR description and code execution on the runner — and the person applying it is reviewing the diff, not the description.

The change here is the standard fix from GitHub's own hardening guidance: pass the value through env: so it arrives as a shell variable instead of being substituted into the script.

        env:
          PR_BODY: ${{ github.event.pull_request.body }}
        run: |
          version=$(echo "$PR_BODY" | grep -oP '## Version \K[0-9]+\.[0-9]+\.[0-9]+')

Behaviour is unchanged — the same grep runs against the same text. Two lines added, one changed.

I checked the other ${{ }} uses in this workflow while I was here; the rest are refs and outputs rather than free text, so I left them alone rather than widen the diff.

Disclosure: I used AI assistance to help spot this and prepare the change, and I verified the workflow and its trigger conditions myself.

…nto the shell

Actions expands ${{ }} into the script text, so a PR body containing $(...) runs
on the runner. Passing it via env: makes it data instead of code.
@kobihikri
kobihikri requested a review from xgreenx as a code owner July 28, 2026 15:27
@fuel-cla-bot

fuel-cla-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

Thanks for the contribution! Before we can merge this, we need @kobihikri to sign the Fuel Labs Contributor License Agreement.

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Workflow-only change with identical functional output; reduces runner code-execution risk from PR descriptions without touching application code.

Overview
Hardens the Create version workflow so the pull request body is no longer expanded into the shell script text.

The Get the version number step now sets PR_BODY from github.event.pull_request.body in env: and runs grep against "$PR_BODY". Version extraction behavior is the same; untrusted description content is treated as data instead of executable script, closing injection via $(...) or backticks in the body when the job runs after the pr release label is applied.

Reviewed by Cursor Bugbot for commit d7b07ef. Bugbot is set up for automated code reviews on this repo. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant