Conversation
…eckout security model in ghbuild.yml Co-authored-by: litlfred <662242+litlfred@users.noreply.github.com>
pull_request_target with split-checkout security model
…_request_target Co-authored-by: litlfred <662242+litlfred@users.noreply.github.com>
pull_request_target with split-checkout security modelpull_request_target with split-checkout security model
Co-authored-by: litlfred <662242+litlfred@users.noreply.github.com>
pull_request_target with split-checkout security modelinputs.ref from ghbuild.yml to eliminate arbitrary-ref injection risk
Co-authored-by: litlfred <662242+litlfred@users.noreply.github.com>
inputs.ref from ghbuild.yml to eliminate arbitrary-ref injection riskinputs.ref and sanitize branch name to prevent GITHUB_ENV injection
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate an arbitrary Git ref injection vector in the reusable GitHub Actions build workflow by removing the ref input and relying on the default event ref, updating the CI and PR preview workflows accordingly.
Changes:
- Removed the
refinput fromghbuild.ymland simplified checkout/branch-detection logic. - Switched
ci.ymlandpr-preview.ymltriggers topull_request_targetand updated deploy gating/conditions. - Updated PR-number detection in
ghbuild.ymlto match the new event type.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/ghbuild.yml | Removes ref input and explicit checkout ref; updates PR detection and branch naming logic. |
| .github/workflows/ci.yml | Switches PR trigger to pull_request_target and adjusts deploy + commit-pot conditions accordingly. |
| .github/workflows/pr-preview.yml | Switches preview trigger to pull_request_target while continuing to call ghbuild.yml for deployment. |
| pull_request_target: | ||
| branches-ignore: |
There was a problem hiding this comment.
Switching this workflow to pull_request_target changes github.ref to the base branch ref for PR runs. With the current concurrency.group: deploy-${{ github.ref }}, all PRs targeting the same base branch will share a concurrency group and can cancel each other (and potentially cancel a concurrent push build on that base branch). Use a PR-unique concurrency key for pull_request_target (e.g., PR number or github.head_ref) while keeping the push behavior unchanged.
| pull_request_target: | ||
| branches-ignore: |
There was a problem hiding this comment.
pull_request_target runs for fork PRs too, but this workflow grants contents: write and pull-requests: write to the called build job. Even if the checkout is “trusted”, this is still elevated permission on attacker-controlled events and can be abused for repo/PR write actions (spam, resource abuse, etc.). Consider skipping fork PRs (as pr-preview does) or reducing permissions for PR-target runs.
|
|
||
| on: | ||
| pull_request: | ||
| pull_request_target: |
There was a problem hiding this comment.
With pull_request_target here and ghbuild.yml now doing a default checkout, the preview deployment will build/deploy the base branch content into a folder named after the PR branch. That makes the preview URL misleading (it won’t contain the PR’s changes). Either restore PR-head checkout in ghbuild.yml for pull_request_target runs (with a safe/split-checkout approach), or keep this workflow on pull_request so the checkout defaults to the PR merge ref.
| pull_request_target: | |
| pull_request: |
| # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ inputs.ref || github.ref }} | ||
|
|
There was a problem hiding this comment.
With actions/checkout@v4 using the default ref, runs invoked via workflow_call from pull_request_target will check out the base branch commit (not the PR head). That means CI/preview builds won’t reflect PR changes, while PR comments/branch metadata still refer to the PR branch. Consider explicitly checking out github.event.pull_request.head.sha for pull_request_target runs (and, if needed, keep the “trusted scripts overlay”/split-checkout mitigation rather than accepting an arbitrary inputs.ref).
… pull_request for ci.yml Merge upstream security changes from PR #312 with our Python-based script injection hardening approach. Key merge decisions: - ci.yml: Keep pull_request trigger (NOT pull_request_target). CI validation must build the PR code, not the base branch. deploy: false on PRs already prevents secrets exposure. - pr-preview.yml: Adopt pull_request_target from upstream with our approve + deploy-preview split (structurally better). Pass PR head SHA via ref input so ghbuild.yml builds the PR code after approval. - ghbuild.yml: Keep our Python scripts approach for sanitization. Move ref input from workflow_dispatch to workflow_call (needed by pr-preview.yml to pass the PR head SHA for checkout). - find_pr.py: Handle both pull_request and pull_request_target events. - resolve_branch.py: Remove INPUT_REF handling — branch name always comes from head_ref or ref_name, never from the checkout ref.
inputs.refallowed callers to supply an arbitrary Git ref, enabling untrusted code execution. After its removal,github.head_ref(user-controlled onpull_request_target) was still written unsanitized into$GITHUB_ENV, allowing newline injection — e.g. a branch namedfeat%0ASECRET=injectedwould addSECRET=injectedto the environment of all subsequent steps.Changes
ghbuild.ymlrefinput fromworkflow_callandworkflow_dispatchinputs.ref != '')ref:fromactions/checkout— uses event context defaultACTUAL_BRANCHbefore writing to$GITHUB_ENV:"pull_request"event check inFind PR number(workflow never fires directly onpull_request)ci.yml/pr-preview.ymlref: ${{ github.event.pull_request.head.sha }}fromghbuild.ymlcall sitesOriginal prompt
Problem
GitHub Actions workflow runs triggered by commits from the Copilot coding agent (e.g., on
copilot/fix-*branches) require manual maintainer approval before running, because the bot'sauthor_associationisCONTRIBUTOR. This blocks CI feedback on every Copilot-agent PR.See: https://github.com/WorldHealthOrganization/smart-base/actions/runs/22950104097
Solution
Switch
ci.ymlandpr-preview.ymlfrompull_requesttopull_request_targettrigger, and implement a split-checkout security model inghbuild.ymlso that:main(input/scripts/) are overlaid on top, so only reviewed code is ever executedThis eliminates the maintainer approval gate for Copilot-agent PRs while maintaining security — untrusted PR code is never executed, only built as data.
Changes required
1.
ci.ymlpull_request:trigger topull_request_target:ref: ${{ github.event.pull_request.head.sha || '' }}toghbuild.ymlgithub.event_name != 'pull_request'togithub.event_name != 'pull_request_target'2.
pr-preview.ymlpull_request:trigger topull_request_target:ghbuild.ymlvia a newrefinput3.
ghbuild.ymlrefas an optional input toworkflow_callref), add a second checkout that usesactions/checkout@v4withref: main,sparse-checkout: input/scripts, andpath: __trusted_scripts__rm -rf input/scripts && cp -r __trusted_scripts__/input/scripts input/scripts && rm -rf __trusted_scripts__pull_request_target(forpushandworkflow_dispatch, the checked-out code is already trusted since the committer has write access)Security model
The key insight: with
pull_request_target, the workflow YAML is read from the base branch (main), so an attacker can't modify the workflow itself. The split-checkout ensuresinput/scripts/*.py(which get executed bypython3) always come frommain. The PR branch only provides data files (FSH, BPMN, XML, YAML, JSON, images) that are processed by the trusted scripts but never executed directly.Files that are executed and must come from
main:input/scripts/*.py— all Python scripts run bypython3Files that are data and safe to come from the PR branch:
input/fsh/**— FSH definitions processed by SUSHIinput/business-processes/*.bpmn— BPMN XML validated by scriptssushi-config.yaml,dak.json— config files read by scriptsinput/images/**,input/pagecontent/**— static contentinput/translations/**— translation filesImportant edge case
When a PR is developing new scripts (e.g., the skills infrastructure PR), those new scripts won't run in CI until merged to
main. This is the correct security behavior — new executable code must be reviewed and merged before it can run with elevated privileges. Content PRs (the common case) work perfectly.NOT in scope
ghbuild.ymlworkflow triggers (it should remainworkflow_call+workflow_dispatchonly)The following is the prior conversation context from the user's chat exploration (may be truncated):
User: https://github.com/WorldHealthOrganization/smart-base/actions/runs/22950104097
this is an action waiting on approval by a maintainer. this was an action triggered by a commit to a feature branch by copilot. what can we do so that the maintainer does not need to approve
Assistant: Good question! Here's what's happening and what you can do about it:
Why the workflow is waiting for approval
The workflow run on
WorldHealthOrganization/smart-basebranchcopilot/fix-invalid-workflow-file(PR #311) is in "Waiting for approval" state. This happens because:copilot-swe-agent) is the commit author, and itsauthor_associationis"CONTRIBUTOR"— notMEMBER/COLLABORATOR/OWNER.Options to fix this (ranked by practicality)
Option 1: Change the org-level fork approval setting ✅ (Recommended)
Since these are not fork PRs — the
copilot/fix-*branches are in the same repo (WorldHealthOrganization/smart-base), not a fork — this shouldn't normally require approval. But the Copilot bot is treated as an outside contributor.A WHO org admin can go to:
This pull request was created from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.