[WIP] Ensure updates are pushing to other orgs and repos#5
[WIP] Ensure updates are pushing to other orgs and repos#5
Conversation
Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
| PR_NUMBER="${{ github.event.pull_request.number }}" | ||
| elif [ "${{ github.event_name }}" == "workflow_run" ]; then | ||
| # Extract PR number from workflow run | ||
| PR_NUMBER=$(gh pr list --json number,headRefName --jq '.[] | select(.headRefName=="${{ github.event.workflow_run.head_branch }}") | .number' | head -1) |
Check failure
Code scanning / CodeQL
Code injection Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 hour ago
In general, to fix this type of problem in GitHub Actions, you should avoid using ${{ ... }} expressions with untrusted values directly inside run: scripts. Instead, pass the value into the step via env: and then consume it using the shell’s variable expansion syntax (e.g., $HEAD_BRANCH) or via process.env in JavaScript. This ensures that the shell does not reinterpret the value as code, regardless of its contents.
For this specific workflow, the risky part is line 41:
PR_NUMBER=$(gh pr list --json number,headRefName --jq '.[] | select(.headRefName=="${{ github.event.workflow_run.head_branch }}") | .number' | head -1)The best fix that preserves existing functionality is:
- Add an environment variable (e.g.,
HEAD_BRANCH) to this step, assigned from${{ github.event.workflow_run.head_branch }}. - Update the
run:script to reference$HEAD_BRANCHinside thejqfilter, using shell variable expansion, while preserving the required quoting so thatjqreceives the correct string and the branch name is not interpreted as code.
Concretely:
- Extend the
env:block in the “Get PR info” step withHEAD_BRANCH: ${{ github.event.workflow_run.head_branch }}. - Change the
PR_NUMBERassignment to use$HEAD_BRANCHinstead of an inlined${{ ... }}. Because the script is single-quoted with an internal double-quoted jq string, the safest approach is to break out of single quotes just around the$HEAD_BRANCHreference: first build the jq filter in a shell variable using proper quoting, then pass that variable togh pr list --jq.
All changes are within .github/workflows/auto-merge.yml, in the shown step.
| @@ -32,13 +32,15 @@ | ||
| id: pr | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} | ||
| run: | | ||
| # Get PR number from the event | ||
| if [ "${{ github.event_name }}" == "pull_request_review" ]; then | ||
| PR_NUMBER="${{ github.event.pull_request.number }}" | ||
| elif [ "${{ github.event_name }}" == "workflow_run" ]; then | ||
| # Extract PR number from workflow run | ||
| PR_NUMBER=$(gh pr list --json number,headRefName --jq '.[] | select(.headRefName=="${{ github.event.workflow_run.head_branch }}") | .number' | head -1) | ||
| JQ_FILTER=".[] | select(.headRefName==\"$HEAD_BRANCH\") | .number" | ||
| PR_NUMBER=$(gh pr list --json number,headRefName --jq "$JQ_FILTER" | head -1) | ||
| else | ||
| echo "No PR found for event type: ${{ github.event_name }}" | ||
| exit 1 |
| - name: Check merge conditions | ||
| id: check | ||
| run: | | ||
| STATE="${{ steps.pr.outputs.state }}" |
Check failure
Code scanning / CodeQL
Code injection Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 hour ago
To fix this, we should stop using GitHub expression syntax (${{ ... }}) directly inside the shell script body for values that originate from workflow inputs/outputs. Instead, we should bind those values to environment variables at the step level (env:) and then reference them using standard shell variable expansion (e.g., $STATE) inside the script. This aligns with GitHub’s guidance and breaks the taint flow into the shell command parser.
Concretely, for the “Check merge conditions” step (lines 67–87), we will:
- Add an
env:section that mapsSTATE,MERGEABLE, andREVIEW_DECISIONto${{ steps.pr.outputs.state }},${{ steps.pr.outputs.mergeable }}, and${{ steps.pr.outputs.review_decision }}respectively. - Simplify the
run:script so it relies solely on$STATE,$MERGEABLE, and$REVIEW_DECISION, removing the inline${{ ... }}assignments in the script body.
No other steps need modification to address this particular alert, and this change preserves existing behavior: the variables in the script will contain exactly the same values as before, but are populated in a safer way by the runner rather than the shell parsing interpolated expressions.
| @@ -66,11 +66,11 @@ | ||
|
|
||
| - name: Check merge conditions | ||
| id: check | ||
| env: | ||
| STATE: ${{ steps.pr.outputs.state }} | ||
| MERGEABLE: ${{ steps.pr.outputs.mergeable }} | ||
| REVIEW_DECISION: ${{ steps.pr.outputs.review_decision }} | ||
| run: | | ||
| STATE="${{ steps.pr.outputs.state }}" | ||
| MERGEABLE="${{ steps.pr.outputs.mergeable }}" | ||
| REVIEW_DECISION="${{ steps.pr.outputs.review_decision }}" | ||
|
|
||
| echo "PR State: $STATE" | ||
| echo "Mergeable: $MERGEABLE" | ||
| echo "Review Decision: $REVIEW_DECISION" |
| id: check | ||
| run: | | ||
| STATE="${{ steps.pr.outputs.state }}" | ||
| MERGEABLE="${{ steps.pr.outputs.mergeable }}" |
Check failure
Code scanning / CodeQL
Code injection Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 hour ago
In general, to fix this class of problems in GitHub Actions, any untrusted or potentially untrusted data that needs to be used in a shell script should be assigned to an environment variable at the workflow level (using ${{ ... }} only in the env: mapping), and then referenced inside the script using the shell’s own variable syntax ($VAR), not ${{ env.VAR }} or similar expression syntax.
For this workflow, the problematic use is in the “Check merge conditions” step, where the script begins with:
run: |
STATE="${{ steps.pr.outputs.state }}"
MERGEABLE="${{ steps.pr.outputs.mergeable }}"
REVIEW_DECISION="${{ steps.pr.outputs.review_decision }}"The safest fix, without altering behavior, is:
- Add an
env:section to that step, mapping:STATE: ${{ steps.pr.outputs.state }}MERGEABLE: ${{ steps.pr.outputs.mergeable }}REVIEW_DECISION: ${{ steps.pr.outputs.review_decision }}
- Remove the three assignment lines inside the
run:block and instead rely on those environment variables directly ($STATE,$MERGEABLE,$REVIEW_DECISION) everywhere in the script, which is already the case for subsequent references.
This keeps functionality identical (the variables have the same names and contents) while complying with the recommended pattern: untrusted values are injected only into environment variables via expressions, and the shell script reads them with native syntax. No additional imports or external dependencies are required, and no other steps need to change.
| @@ -66,11 +66,11 @@ | ||
|
|
||
| - name: Check merge conditions | ||
| id: check | ||
| env: | ||
| STATE: ${{ steps.pr.outputs.state }} | ||
| MERGEABLE: ${{ steps.pr.outputs.mergeable }} | ||
| REVIEW_DECISION: ${{ steps.pr.outputs.review_decision }} | ||
| run: | | ||
| STATE="${{ steps.pr.outputs.state }}" | ||
| MERGEABLE="${{ steps.pr.outputs.mergeable }}" | ||
| REVIEW_DECISION="${{ steps.pr.outputs.review_decision }}" | ||
|
|
||
| echo "PR State: $STATE" | ||
| echo "Mergeable: $MERGEABLE" | ||
| echo "Review Decision: $REVIEW_DECISION" |
| run: | | ||
| STATE="${{ steps.pr.outputs.state }}" | ||
| MERGEABLE="${{ steps.pr.outputs.mergeable }}" | ||
| REVIEW_DECISION="${{ steps.pr.outputs.review_decision }}" |
Check failure
Code scanning / CodeQL
Code injection Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 hour ago
General approach: Avoid using ${{ steps.pr.outputs.review_decision }} directly inside the run: script. Instead, pass the value into the step via env: and use standard shell variable expansion ($REVIEW_DECISION) within the script. This aligns with the recommended pattern of copying untrusted expressions into environment variables and only using native shell syntax in the command body.
Concrete fix for this workflow:
-
In the
Check merge conditionsstep (lines 67–96), add anenv:section that maps a new environment variable (e.g.,STATE,MERGEABLE,REVIEW_DECISION) from the step outputs:
REVIEW_DECISION: ${{ steps.pr.outputs.review_decision }}. -
In the
run:script of that same step, stop reassigning these variables using${{ ... }}. Instead, rely on the environment variables already populated byenv:. So remove or simplify the lines:STATE="${{ steps.pr.outputs.state }}" MERGEABLE="${{ steps.pr.outputs.mergeable }}" REVIEW_DECISION="${{ steps.pr.outputs.review_decision }}"
and use
$STATE,$MERGEABLE, and$REVIEW_DECISIONdirectly. -
No other steps need to change, because the CodeQL alert is specific to
${{ steps.pr.outputs.review_decision }}at line 72.
This preserves behavior (the logic still checks the same values) while ensuring no GitHub Actions expressions are present inside the actual shell script body.
| @@ -66,11 +66,11 @@ | ||
|
|
||
| - name: Check merge conditions | ||
| id: check | ||
| env: | ||
| STATE: ${{ steps.pr.outputs.state }} | ||
| MERGEABLE: ${{ steps.pr.outputs.mergeable }} | ||
| REVIEW_DECISION: ${{ steps.pr.outputs.review_decision }} | ||
| run: | | ||
| STATE="${{ steps.pr.outputs.state }}" | ||
| MERGEABLE="${{ steps.pr.outputs.mergeable }}" | ||
| REVIEW_DECISION="${{ steps.pr.outputs.review_decision }}" | ||
|
|
||
| echo "PR State: $STATE" | ||
| echo "Mergeable: $MERGEABLE" | ||
| echo "Review Decision: $REVIEW_DECISION" |
| echo "🎯 Auto-merge Summary" | ||
| echo "" | ||
| echo "PR: #${{ steps.pr.outputs.pr_number }}" | ||
| echo "State: ${{ steps.pr.outputs.state }}" |
Check failure
Code scanning / CodeQL
Code injection Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 hour ago
General fix approach: avoid using ${{ ... }} directly inside shell scripts for values derived from external data. Instead, assign those expression values to environment variables in the step configuration (env:) and then reference them in the script using the shell’s native $VAR syntax.
Best concrete fix here: in the Summary step, move all ${{ steps.* }} and ${{ job.status }} references out of the run: body into env: variables, then reference them with $VAR inside the script. This removes the vulnerable pattern and also addresses any related variants (not only state but pr_number, can_merge, job.status) in one place.
Specifically, in .github/workflows/auto-merge.yml:
- For the
Summarystep (lines 124–132), add anenv:section with variables likeSUMMARY_PR_NUMBER,SUMMARY_STATE,SUMMARY_CAN_MERGE, andSUMMARY_JOB_STATUS, each assigned from the corresponding${{ ... }}expression. - Update the
run:block to echo using$SUMMARY_PR_NUMBER,$SUMMARY_STATE,$SUMMARY_CAN_MERGE, and$SUMMARY_JOB_STATUSinstead of interpolated expressions.
No new methods or imports are required; this is purely a YAML/workflow configuration change.
| @@ -123,10 +123,15 @@ | ||
|
|
||
| - name: Summary | ||
| if: always() | ||
| env: | ||
| SUMMARY_PR_NUMBER: ${{ steps.pr.outputs.pr_number }} | ||
| SUMMARY_STATE: ${{ steps.pr.outputs.state }} | ||
| SUMMARY_CAN_MERGE: ${{ steps.check.outputs.can_merge }} | ||
| SUMMARY_JOB_STATUS: ${{ job.status }} | ||
| run: | | ||
| echo "🎯 Auto-merge Summary" | ||
| echo "" | ||
| echo "PR: #${{ steps.pr.outputs.pr_number }}" | ||
| echo "State: ${{ steps.pr.outputs.state }}" | ||
| echo "Can merge: ${{ steps.check.outputs.can_merge }}" | ||
| echo "Status: ${{ job.status }}" | ||
| echo "PR: #$SUMMARY_PR_NUMBER" | ||
| echo "State: $SUMMARY_STATE" | ||
| echo "Can merge: $SUMMARY_CAN_MERGE" | ||
| echo "Status: $SUMMARY_JOB_STATUS" |
Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
…ocumentation Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46f5a4b8ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ -z "$PR_NUMBER" ]; then | ||
| echo "No PR number found for branch ${{ github.event.workflow_run.head_branch }}" | ||
| exit 1 |
There was a problem hiding this comment.
Guard workflow_run when no PR exists
On workflow_run events, CI also runs for direct pushes to main/develop (see .github/workflows/ci.yml on.push.branches), so there is often no associated PR. In that case PR_NUMBER will be empty and this step exits 1, causing the auto-merge workflow to fail on every push/merge to those branches. Consider skipping the job when github.event.workflow_run.pull_requests is empty or explicitly gating workflow_run to PR-triggered runs to avoid persistent failures.
Useful? React with 👍 / 👎.
| if [ "$STATE" == "OPEN" ] && \ | ||
| [ "$MERGEABLE" == "MERGEABLE" ] && \ | ||
| [ "$REVIEW_DECISION" == "APPROVED" ]; then |
There was a problem hiding this comment.
Enforce base branch before auto-merge
The merge gate only checks state, mergeable, and reviewDecision and never verifies the PR’s base branch. Because CI also runs on develop, an approved PR targeting develop will satisfy these checks and be auto-merged, despite the workflow’s stated intent to merge only to main. Add a base-branch condition (e.g., github.event.pull_request.base.ref == 'main' or equivalent for workflow_run) to prevent unintended merges to non-main branches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive sync system to push updates from the central .github repository (The Bridge) to other BlackRoad organization repositories. The implementation adds automated sync workflows, comprehensive testing, auto-merge functionality, and documentation.
Changes:
- Adds sync-to-orgs workflow that dispatches repository events to active organization repositories
- Implements auto-merge workflow to automatically merge approved PRs after CI passes
- Integrates sync testing into the CI pipeline
- Provides comprehensive documentation and receiver workflow templates
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_sync.py | Test suite validating sync workflow configuration and registry structure |
| tests/test_integration.py | End-to-end integration test simulating the complete sync flow |
| .github/workflows/sync-to-orgs.yml | Main workflow dispatching sync events to target organization repositories |
| .github/workflows/auto-merge.yml | Workflow automating PR merges after approval and successful CI |
| .github/workflows/ci.yml | Updated CI pipeline including sync functionality tests |
| templates/workflows/sync-receiver.yml | Template workflow for target repositories to receive and process sync events |
| docs/SYNC.md | Comprehensive documentation covering sync architecture, usage, and troubleshooting |
| README.md | Updated overview with sync features and quick start instructions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/auto-merge.yml
Outdated
| echo "No PR number found for branch ${{ github.event.workflow_run.head_branch }}" | ||
| exit 1 |
There was a problem hiding this comment.
The workflow will fail with "exit 1" when no PR number is found for a branch in workflow_run events. However, this is normal behavior when CI runs on direct commits to main (not from a PR). The workflow should handle this case gracefully by exiting early without error, rather than failing the entire workflow. Consider checking if this is a PR-related run before attempting to find the PR number, or exit with success (exit 0) when no PR is found.
| echo "No PR number found for branch ${{ github.event.workflow_run.head_branch }}" | |
| exit 1 | |
| if [ "${{ github.event_name }}" == "workflow_run" ]; then | |
| echo "No PR number found for branch ${{ github.event.workflow_run.head_branch }}; this workflow_run is not associated with a PR. Exiting successfully." | |
| exit 0 | |
| else | |
| echo "No PR number found for event type: ${{ github.event_name }}" | |
| exit 1 | |
| fi |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Should trigger on push to main | ||
| if "push" in triggers: | ||
| branches = triggers["push"].get("branches", []) |
There was a problem hiding this comment.
If the workflow YAML ever omits the push trigger entirely, this test won't record an error because it only validates branches when "push" in triggers. Consider adding an else path that calls self.error(...) when the push trigger is missing so regressions are caught.
| sync: | ||
| name: Sync Updates | ||
| runs-on: ubuntu-latest | ||
|
|
There was a problem hiding this comment.
This template pushes commits back to the default branch, but it doesn't request contents: write permissions. In repos/orgs where the default GITHUB_TOKEN permission is read-only, the git push step will fail. Add explicit workflow/job permissions: contents: write (and keep pull-requests etc. off unless needed).
| permissions: | |
| contents: write | |
| # Output for next steps | ||
| with open('$GITHUB_OUTPUT', 'a') as f: | ||
| f.write(f'orgs={json.dumps(orgs)}\\n') |
There was a problem hiding this comment.
In this Python step, open('$GITHUB_OUTPUT', 'a') writes to a literal file named $GITHUB_OUTPUT rather than the Actions output file path. As a result, steps.registry.outputs.orgs will be empty and the dispatch step will likely fail when it tries to json.loads the missing/empty env var. Use the GITHUB_OUTPUT environment variable (e.g., via os.environ['GITHUB_OUTPUT']) or write the output from the shell instead.
|
|
||
| - name: Dispatch to target orgs | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.DISPATCH_TOKEN || secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
secrets.GITHUB_TOKEN is repository-scoped and typically cannot call repository_dispatch on other repositories (even within the same org). Falling back to it will make sync appear configured but fail at runtime; consider requiring DISPATCH_TOKEN (fail fast if missing) or only using the fallback when dispatching to the current repo.
| GITHUB_TOKEN: ${{ secrets.DISPATCH_TOKEN || secrets.GITHUB_TOKEN }} | |
| GITHUB_TOKEN: ${{ secrets.DISPATCH_TOKEN }} |
| if failures: | ||
| print(f'⚠️ {len(failures)} dispatch(es) failed:') | ||
| for failure in failures: | ||
| print(f' - {failure}') | ||
| print('') |
There was a problem hiding this comment.
The dispatch script collects failures but never exits non-zero when dispatches fail, so the workflow can report success even when nothing was synced. Consider failing the job when failures is non-empty (and potentially exempting dry_run) so broken dispatches surface immediately.
| if: | | ||
| (github.event_name == 'pull_request_review' && github.event.review.state == 'approved') || | ||
| (github.event_name == 'workflow_run' && github.event.workflow_run.conclusion == 'success') | ||
|
|
There was a problem hiding this comment.
The job condition runs on any successful workflow_run of CI, including CI runs triggered by direct pushes to main/develop (where there is no PR). That will cause this workflow to error (no PR found) and may also risk selecting the wrong PR when matching by branch name. Gate workflow_run handling to PR-triggered runs only (e.g., require github.event.workflow_run.event == 'pull_request' and/or a non-empty workflow_run.pull_requests list), and ensure the PR targets main before enabling auto-merge.
| # Extract PR number from workflow run | ||
| PR_NUMBER=$(gh pr list --json number,headRefName --jq '.[] | select(.headRefName=="${{ github.event.workflow_run.head_branch }}") | .number' | head -1) |
There was a problem hiding this comment.
For workflow_run, deriving the PR number by listing PRs and matching headRefName to workflow_run.head_branch is ambiguous and can return the wrong PR if multiple PRs share the same head branch name (or return nothing and fail). Prefer using the PR number(s) provided in the workflow_run event payload (when present) to identify the exact PR that triggered CI.
| # Extract PR number from workflow run | |
| PR_NUMBER=$(gh pr list --json number,headRefName --jq '.[] | select(.headRefName=="${{ github.event.workflow_run.head_branch }}") | .number' | head -1) | |
| # Extract PR number directly from workflow_run payload | |
| if [ -n "${{ github.event.workflow_run.pull_requests[0].number }}" ]; then | |
| PR_NUMBER="${{ github.event.workflow_run.pull_requests[0].number }}" | |
| else | |
| echo "Workflow run is not associated with a pull request." | |
| exit 1 | |
| fi |
| with open('routes/registry.yaml') as f: | ||
| reg = yaml.safe_load(f) | ||
| active = [code for code, org in reg['orgs'].items() if org.get('status') == 'active'] | ||
| print(f'Active orgs: {', '.join(active)}') |
There was a problem hiding this comment.
This Python example is syntactically invalid because the f-string contains nested single quotes: print(f'Active orgs: {', '.join(active)}'). Adjust the quoting (or build the joined string separately) so the snippet can be copied/pasted and run as shown.
| print(f'Active orgs: {', '.join(active)}') | |
| joined = ', '.join(active) | |
| print(f'Active orgs: {joined}') |
Implementation Plan: Test & Ensure Updates Push to Orgs
Understanding
This repository (
.github) is a central coordination hub for 15 BlackRoad organizations. The implementation adds:Implementation Checklist
sync-to-orgs.ymlworkflow that pushes workflow templatesauto-merge.ymlfor approved PRsOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.