fix: dispatch checks for automation PRs - #32
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds manual workflow dispatch support, introduces a shared script that dispatches and validates fresh PR checks, updates protected-controls PR resolution, and replaces direct check polling in maintenance workflows. Documentation, protected-path configuration, and maintenance tests cover the new coordination flow. Deterministic PR check dispatch
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MaintenanceWorkflow
participant DispatchPRChecks
participant GitHubActions
participant ChecksAPI
MaintenanceWorkflow->>DispatchPRChecks: pass PR number, check name, and output path
DispatchPRChecks->>GitHubActions: dispatch ci.yml and protected-controls.yml
DispatchPRChecks->>ChecksAPI: poll newly created check runs
ChecksAPI-->>DispatchPRChecks: return check conclusions
DispatchPRChecks-->>MaintenanceWorkflow: write check result JSON
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/protected-controls.yml:
- Around line 23-45: Update the “Resolve exact pull request state” step to pass
github.ref, github.sha, and github.event_name through its env block, then
reference those environment variables in the bash conditionals instead of
embedding expressions directly in run. Preserve the existing workflow_dispatch
validation behavior.
In `@scripts/dispatch-pr-checks`:
- Around line 45-49: Replace the bare assertions in the dispatch validation
block with checks that emit a clear diagnostic identifying the failed condition
before exiting. Preserve the existing validation rules for state, base_ref,
head_repository, head_ref, and head_sha, and ensure failures remain fatal under
the script’s current control flow.
- Around line 63-90: Update the polling loop around the primary and protected
check-run evaluations so each non-success completed check is detected and
reported independently, without requiring both $primary and $protected to exist.
Preserve the success/output path only when both checks are present and completed
successfully, while continuing to wait for a missing check unless the existing
deadline is reached.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44de6624-92d7-4deb-9aa0-c0fed4bfcff5
📒 Files selected for processing (9)
.github/workflows/ci.yml.github/workflows/maintenance-implementation.yml.github/workflows/maintenance-release.yml.github/workflows/maintenance-watch.yml.github/workflows/protected-controls.ymldocs/repository-settings.mdmaintenance/protected-paths.jsonscripts/dispatch-pr-checkstests/test_maintenance.py
| - name: Resolve exact pull request state | ||
| id: pr | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} | ||
| run: | | ||
| [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]] | ||
| gh api "repos/$REPOSITORY/pulls/$PR_NUMBER" > "$RUNNER_TEMP/pr.json" | ||
| test "$(jq -r .state "$RUNNER_TEMP/pr.json")" = "open" | ||
| test "$(jq -r .base.ref "$RUNNER_TEMP/pr.json")" = "main" | ||
| if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then | ||
| test "$(jq -r .head.sha "$RUNNER_TEMP/pr.json")" = "${{ github.sha }}" | ||
| test "refs/heads/$(jq -r .head.ref "$RUNNER_TEMP/pr.json")" = "${{ github.ref }}" | ||
| fi | ||
| { | ||
| echo "number=$PR_NUMBER" | ||
| echo "head_sha=$(jq -r .head.sha "$RUNNER_TEMP/pr.json")" | ||
| echo "base_sha=$(jq -r .base.sha "$RUNNER_TEMP/pr.json")" | ||
| echo "head_ref=$(jq -r .head.ref "$RUNNER_TEMP/pr.json")" | ||
| echo "head_repository=$(jq -r .head.repo.full_name "$RUNNER_TEMP/pr.json")" | ||
| echo "author=$(jq -r .user.login "$RUNNER_TEMP/pr.json")" | ||
| } >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files .github/workflows/protected-controls.yml
echo
echo "== targeted workflow excerpt =="
if [ -f .github/workflows/protected-controls.yml ]; then
sed -n '1,120p' .github/workflows/protected-controls.yml | cat -n
fi
echo
echo "== occurrences in workflow =="
rg -n 'github\.(ref|sha|event_name)|PR_NUMBER|pr_number|workflow_dispatch|pull_request' .github/workflows/protected-controls.yml || trueRepository: Bigpixelrocket/php-bin
Length of output: 6379
🌐 Web query:
GitHub Actions protected branch rules enforce pull requests created on untrusted forks cannot be built with workflow_dispatch
💡 Result:
The statement that GitHub Actions protected branch rules specifically enforce that pull requests from untrusted forks cannot be built with workflow_dispatch is not accurate as a general rule, but it aligns with the broader security mechanisms GitHub employs to manage manual triggers and untrusted code [1][2]. Workflow_dispatch is a manual trigger that requires the workflow file to exist on the repository's default branch [3]. Because it is a manual trigger, it is not tied to the automated pull request lifecycle in the way events like pull_request or pull_request_target are [3]. Key security context: 1. Manual Trigger Controls: GitHub provides enterprise-level and repository-level policies that allow administrators to limit workflow_dispatch triggers specifically to maintainers or users with specific permissions [1][2]. This is designed to prevent manual-trigger abuse, where untrusted identities might otherwise attempt to initiate workflows [1][2]. 2. Fork Pull Request Protections: The primary concern with untrusted forks is the "pwn request" pattern, where malicious code from a fork is executed in a privileged context (e.g., using pull_request_target) [4]. GitHub mitigates this by restricting pull_request_target and by ensuring that actions/checkout v7 and later refuses to check out fork pull request code in privileged contexts by default [5][4]. 3. Event Restrictions: GitHub allows organizations to enforce event rules that control which triggers are permitted [1][2]. While you can restrict workflow_dispatch globally or by user, the restriction is not inherently linked to "pull requests created on untrusted forks" by default; rather, it is a policy-driven control to prevent unauthorized users from executing workflows [1][2]. In summary, while you cannot "pull" a workflow_dispatch trigger from a fork (since it must reside on the default branch), you can and should use GitHub Actions policies to restrict who is authorized to manually trigger workflows, thereby preventing untrusted actors from leveraging this capability [1][2][3].
Citations:
- 1: https://docs.github.com/en/enterprise-cloud@latest/admin/enforcing-policies/enforcing-policies-for-your-enterprise/actions-policies/workflow-execution-protections
- 2: https://github.blog/changelog/2026-06-18-control-who-and-what-triggers-github-actions-workflows/
- 3: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow
- 4: https://github.blog/changelog/2026-06-18-safer-pull_request_target-defaults-for-github-actions-checkout/
- 5: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
🌐 Web query:
GitHub Pull Request API head repo full_name base sha head sha fields documentation
💡 Result:
The GitHub REST API provides detailed information about pull requests, including objects for head and base that contain specific branch and commit data [1][2]. When retrieving a single pull request via the GET /repos/{owner}/{repo}/pulls/{pull_number} endpoint, the response object includes nested head and base objects [1][2]. The relevant fields within these objects are: base object: - base.ref: The name of the base branch. - base.sha: The SHA of the latest commit on the base branch at the time the pull request was created or last updated [2]. - base.repo.full_name: The full name of the repository where the base branch resides (e.g., octocat/Hello-World) [2]. head object: - head.ref: The name of the head branch (the branch being merged). - head.sha: The SHA of the latest commit on the head branch [2]. - head.repo.full_name: The full name of the repository where the head branch resides [2]. These fields are essential for identifying the exact state and source of the branches involved in the pull request [2]. Users should consult the official GitHub REST API documentation for the most current schema definitions, as fields may evolve [1][3].
Citations:
- 1: https://docs.github.com/en/rest/pulls/pulls
- 2: https://www.kenmuse.com/blog/the-many-shas-of-a-github-pull-request/
- 3: https://docs.github.com/en/rest/pulls
🌐 Web query:
GitHub Actions branch name special characters backticks semicolon shell command injection github.ref
💡 Result:
Using branch names or tag names that contain special characters (such as backticks, semicolons, quotes, or dollar signs) in GitHub Actions workflows creates a severe security risk known as shell command injection [1][2][3]. This vulnerability occurs when untrusted context variables—such as github.ref, github.head_ref, or github.event.pull_request.head.ref—are directly interpolated into shell scripts within a run: block [1][4][5]. Because GitHub Actions evaluates the ${{ }} expression and substitutes the raw string before the shell executes the command, a malicious branch name (e.g., a branch named test"; ls
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: GHSA-8v8w-v8xg-79rf
- 3: GHSA-gq52-6phf-x2r6
- 4: Security: GitHub Actions script injection via untrusted PR contexts oxsecurity/megalinter#7657
- 5: https://phpunit.expert/articles/hardening-github-actions-workflows.html
- 6: https://github.blog/security/vulnerability-research/how-to-catch-github-actions-workflow-injections-before-attackers-do/
- 7: https://dev.to/nshekhawat/wfguard-a-github-actions-supply-chain-auditor-3obe
Pass github context values through env before using them in the runner script.
${{ github.ref }}, ${{ github.sha }}, and ${{ github.event_name }} are expanded before the bash script runs, so a malicious branch/ref payload can break out of shell syntax when used directly inside run:. Define these as env vars from the expression and reference the env vars in the script, as already done for PR_NUMBER.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 36-36: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/protected-controls.yml around lines 23 - 45, Update the
“Resolve exact pull request state” step to pass github.ref, github.sha, and
github.event_name through its env block, then reference those environment
variables in the bash conditionals instead of embedding expressions directly in
run. Preserve the existing workflow_dispatch validation behavior.
Source: Linters/SAST tools
| [[ "$state" == "open" ]] | ||
| [[ "$base_ref" == "main" ]] | ||
| [[ "$head_repository" == "$repository" ]] | ||
| [[ "$head_ref" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] | ||
| [[ "$head_sha" =~ ^[0-9a-f]{40}$ ]] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add diagnostics to the hard assertions.
These bare [[ ]] checks rely on set -e to abort on failure, but produce no error message — a failed dispatch here just dies with no explanation in the CI log.
🩹 Proposed fix
-[[ "$state" == "open" ]]
-[[ "$base_ref" == "main" ]]
-[[ "$head_repository" == "$repository" ]]
-[[ "$head_ref" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]]
-[[ "$head_sha" =~ ^[0-9a-f]{40}$ ]]
+[[ "$state" == "open" ]] || { echo "PR #$pr_number is not open." >&2; exit 1; }
+[[ "$base_ref" == "main" ]] || { echo "PR #$pr_number does not target main." >&2; exit 1; }
+[[ "$head_repository" == "$repository" ]] || { echo "PR #$pr_number head is not same-repository." >&2; exit 1; }
+[[ "$head_ref" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] || { echo "PR #$pr_number head ref has an unexpected shape." >&2; exit 1; }
+[[ "$head_sha" =~ ^[0-9a-f]{40}$ ]] || { echo "PR #$pr_number head sha has an unexpected shape." >&2; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [[ "$state" == "open" ]] | |
| [[ "$base_ref" == "main" ]] | |
| [[ "$head_repository" == "$repository" ]] | |
| [[ "$head_ref" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] | |
| [[ "$head_sha" =~ ^[0-9a-f]{40}$ ]] | |
| [[ "$state" == "open" ]] || { echo "PR #$pr_number is not open." >&2; exit 1; } | |
| [[ "$base_ref" == "main" ]] || { echo "PR #$pr_number does not target main." >&2; exit 1; } | |
| [[ "$head_repository" == "$repository" ]] || { echo "PR #$pr_number head is not same-repository." >&2; exit 1; } | |
| [[ "$head_ref" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] || { echo "PR #$pr_number head ref has an unexpected shape." >&2; exit 1; } | |
| [[ "$head_sha" =~ ^[0-9a-f]{40}$ ]] || { echo "PR #$pr_number head sha has an unexpected shape." >&2; exit 1; } |
🤖 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 `@scripts/dispatch-pr-checks` around lines 45 - 49, Replace the bare assertions
in the dispatch validation block with checks that emit a clear diagnostic
identifying the failed condition before exiting. Preserve the existing
validation rules for state, base_ref, head_repository, head_ref, and head_sha,
and ensure failures remain fatal under the script’s current control flow.
| deadline=$((SECONDS + 900)) | ||
| latest="" | ||
| while ((SECONDS < deadline)); do | ||
| latest="$(gh api -H "Accept: application/vnd.github+json" "$checks_url")" | ||
| primary="$(jq -c --arg name "$primary_check" --argjson floor "$primary_before" \ | ||
| '[.check_runs[] | select(.name==$name and .id>$floor)] | max_by(.id) // empty' <<<"$latest")" | ||
| protected="$(jq -c --argjson floor "$protected_before" \ | ||
| '[.check_runs[] | select(.name=="Protected controls" and .id>$floor)] | max_by(.id) // empty' <<<"$latest")" | ||
|
|
||
| if [[ -n "$primary" && -n "$protected" ]]; then | ||
| failed="$(jq -r -s '[.[] | select(.status=="completed" and .conclusion!="success") | .details_url] | join("\n")' \ | ||
| <<<"$primary"$'\n'"$protected")" | ||
| if [[ -n "$failed" ]]; then | ||
| echo "A dispatched required check failed:" >&2 | ||
| echo "$failed" >&2 | ||
| exit 1 | ||
| fi | ||
| if jq -e -s 'all(.[]; .status=="completed" and .conclusion=="success")' \ | ||
| <<<"$primary"$'\n'"$protected" >/dev/null; then | ||
| mkdir -p "$(dirname "$output")" | ||
| jq -s --arg head "$head_sha" \ | ||
| 'map({name, bucket:"pass", link:.details_url, headSha:$head, checkRunId:.id})' \ | ||
| <<<"$primary"$'\n'"$protected" > "$output" | ||
| exit 0 | ||
| fi | ||
| fi | ||
| sleep 5 | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Failure detection is gated behind both checks appearing, masking/delaying real failures.
The failed/success evaluation at lines 72-88 only runs if [[ -n "$primary" && -n "$protected" ]]. If one check-run fails or errors out quickly while the other hasn't been created yet (e.g. protected-controls.yml never starts due to a permissions/infra issue), this script silently ignores the already-known failure and spins for the full 900s before reporting a generic timeout — hiding the real cause and wasting up to 15 minutes on every one of the 5 call sites (maintenance-implementation.yml x2, maintenance-release.yml, maintenance-watch.yml x2).
🐛 Proposed fix: evaluate failure independently of whether both checks have appeared
- if [[ -n "$primary" && -n "$protected" ]]; then
- failed="$(jq -r -s '[.[] | select(.status=="completed" and .conclusion!="success") | .details_url] | join("\n")' \
- <<<"$primary"$'\n'"$protected")"
- if [[ -n "$failed" ]]; then
- echo "A dispatched required check failed:" >&2
- echo "$failed" >&2
- exit 1
- fi
- if jq -e -s 'all(.[]; .status=="completed" and .conclusion=="success")' \
- <<<"$primary"$'\n'"$protected" >/dev/null; then
+ present=()
+ [[ -n "$primary" ]] && present+=("$primary")
+ [[ -n "$protected" ]] && present+=("$protected")
+ if ((${`#present`[@]})); then
+ failed="$(printf '%s\n' "${present[@]}" | jq -r -s \
+ '[.[] | select(.status=="completed" and .conclusion!="success") | .details_url] | join("\n")')"
+ if [[ -n "$failed" ]]; then
+ echo "A dispatched required check failed:" >&2
+ echo "$failed" >&2
+ exit 1
+ fi
+ fi
+ if [[ -n "$primary" && -n "$protected" ]]; then
+ if jq -e -s 'all(.[]; .status=="completed" and .conclusion=="success")' \
+ <<<"$primary"$'\n'"$protected" >/dev/null; then
mkdir -p "$(dirname "$output")"
jq -s --arg head "$head_sha" \
'map({name, bucket:"pass", link:.details_url, headSha:$head, checkRunId:.id})' \
<<<"$primary"$'\n'"$protected" > "$output"
exit 0
fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| deadline=$((SECONDS + 900)) | |
| latest="" | |
| while ((SECONDS < deadline)); do | |
| latest="$(gh api -H "Accept: application/vnd.github+json" "$checks_url")" | |
| primary="$(jq -c --arg name "$primary_check" --argjson floor "$primary_before" \ | |
| '[.check_runs[] | select(.name==$name and .id>$floor)] | max_by(.id) // empty' <<<"$latest")" | |
| protected="$(jq -c --argjson floor "$protected_before" \ | |
| '[.check_runs[] | select(.name=="Protected controls" and .id>$floor)] | max_by(.id) // empty' <<<"$latest")" | |
| if [[ -n "$primary" && -n "$protected" ]]; then | |
| failed="$(jq -r -s '[.[] | select(.status=="completed" and .conclusion!="success") | .details_url] | join("\n")' \ | |
| <<<"$primary"$'\n'"$protected")" | |
| if [[ -n "$failed" ]]; then | |
| echo "A dispatched required check failed:" >&2 | |
| echo "$failed" >&2 | |
| exit 1 | |
| fi | |
| if jq -e -s 'all(.[]; .status=="completed" and .conclusion=="success")' \ | |
| <<<"$primary"$'\n'"$protected" >/dev/null; then | |
| mkdir -p "$(dirname "$output")" | |
| jq -s --arg head "$head_sha" \ | |
| 'map({name, bucket:"pass", link:.details_url, headSha:$head, checkRunId:.id})' \ | |
| <<<"$primary"$'\n'"$protected" > "$output" | |
| exit 0 | |
| fi | |
| fi | |
| sleep 5 | |
| done | |
| deadline=$((SECONDS + 900)) | |
| latest="" | |
| while ((SECONDS < deadline)); do | |
| latest="$(gh api -H "Accept: application/vnd.github+json" "$checks_url")" | |
| primary="$(jq -c --arg name "$primary_check" --argjson floor "$primary_before" \ | |
| '[.check_runs[] | select(.name==$name and .id>$floor)] | max_by(.id) // empty' <<<"$latest")" | |
| protected="$(jq -c --argjson floor "$protected_before" \ | |
| '[.check_runs[] | select(.name=="Protected controls" and .id>$floor)] | max_by(.id) // empty' <<<"$latest")" | |
| present=() | |
| [[ -n "$primary" ]] && present+=("$primary") | |
| [[ -n "$protected" ]] && present+=("$protected") | |
| if ((${`#present`[@]})); then | |
| failed="$(printf '%s\n' "${present[@]}" | jq -r -s \ | |
| '[.[] | select(.status=="completed" and .conclusion!="success") | .details_url] | join("\n")')" | |
| if [[ -n "$failed" ]]; then | |
| echo "A dispatched required check failed:" >&2 | |
| echo "$failed" >&2 | |
| exit 1 | |
| fi | |
| fi | |
| if [[ -n "$primary" && -n "$protected" ]]; then | |
| if jq -e -s 'all(.[]; .status=="completed" and .conclusion=="success")' \ | |
| <<<"$primary"$'\n'"$protected" >/dev/null; then | |
| mkdir -p "$(dirname "$output")" | |
| jq -s --arg head "$head_sha" \ | |
| 'map({name, bucket:"pass", link:.details_url, headSha:$head, checkRunId:.id})' \ | |
| <<<"$primary"$'\n'"$protected" > "$output" | |
| exit 0 | |
| fi | |
| fi | |
| sleep 5 | |
| done |
🤖 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 `@scripts/dispatch-pr-checks` around lines 63 - 90, Update the polling loop
around the primary and protected check-run evaluations so each non-success
completed check is detected and reported independently, without requiring both
$primary and $protected to exist. Preserve the success/output path only when
both checks are present and completed successfully, while continuing to wait for
a missing check unless the existing deadline is reached.
1 participant
Repairs deterministic PR coordination for branches and PRs created with GITHUB_TOKEN. Explicitly dispatches CI and protected controls at the exact PR head, accepts only new successful check runs, applies the path to every php-bin automation PR, and adds regression coverage.\n\nVerified locally: php-bin suite (18 tests); cross-repository A00-A20 report digest sha256:4c2984c7bc7088f54470778f710557486a870d1bd244f6229ad2ba25cc6514d8.
Summary by CodeRabbit
New Features
Documentation
Tests