-
Notifications
You must be signed in to change notification settings - Fork 34
feat: enable GitLab CI for fork PRs from org members #2205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Split GitLab CI triggering into separate workflows using workflow_run: - trigger-gitlab-ci.yml: Triggers when main workflow succeeds - cancel-gitlab-ci.yml: Cancels pipeline when main workflow fails Security model: - Main workflow runs in PR/fork context (no custom secrets) - Trigger workflows run in base repo context (has secrets) - Author association checked (MEMBER/OWNER/COLLABORATOR) - workflow_run runs on default branch (never executes PR code) Pipeline cancellation uses artifacts: - Trigger workflow uploads pipeline ID with workflow_run.id as key - Cancel workflow downloads artifact to find exact pipeline to cancel - Reliable, deterministic, no heuristics needed
|
@veprbl This is a first step towards improving our security posture (doesn't address the docs etc). Adding github members without write access would make me feel a lot more comfortable about all this. |
| run: | | ||
| curl --request POST \ | ||
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" |
Check failure
Code scanning / CodeQL
Code injection Critical
${ steps.extract-pipeline.outputs.pipeline_id }
workflow_run
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix this code injection vulnerability, move the untrusted value ${{ steps.extract-pipeline.outputs.pipeline_id }} into an environment variable by setting it in the env: section of the step. Then, inside the shell script (run:), read the value using shell-native syntax ($PIPELINE_ID). This avoids interpreting the value as code, even if it contains malicious characters, because shell variable expansion does not execute code contained within the variable. You only need to change the affected - name: Cancel GitLab pipeline step (lines 75–81): move the extraction of pipeline_id to an environment variable and reference it in the curl command using $PIPELINE_ID rather than ${{ ... }}. No extra package imports are needed.
-
Copy modified lines R77-R78 -
Copy modified lines R82-R83
| @@ -74,8 +74,10 @@ | ||
|
|
||
| - name: Cancel GitLab pipeline | ||
| if: steps.extract-pipeline.outputs.pipeline_id != '' | ||
| env: | ||
| PIPELINE_ID: ${{ steps.extract-pipeline.outputs.pipeline_id }} | ||
| run: | | ||
| curl --request POST \ | ||
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/$PIPELINE_ID/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline $PIPELINE_ID" |
| curl --request POST \ | ||
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" |
Check failure
Code scanning / CodeQL
Code injection Critical
${ steps.extract-pipeline.outputs.pipeline_id }
workflow_run
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To securely pass the pipeline ID to the shell without code injection risk, we should first assign the value of ${{ steps.extract-pipeline.outputs.pipeline_id }} to an environment variable in the workflow step's env: section. Then, in the run: block, reference the environment variable using standard shell variable syntax ($VARIABLE) rather than ${{ ... }}. This prevents the shell from evaluating untrusted contents as code. Only the affected run step (the one running curl) needs to be edited; move all dynamic expressions from the command into env: and use shell $PIPELINE_ID variable in the URL. No extra imports or definitions are needed, just appropriate restructuring of the YAML run step.
-
Copy modified lines R77-R78 -
Copy modified lines R82-R83
| @@ -74,8 +74,10 @@ | ||
|
|
||
| - name: Cancel GitLab pipeline | ||
| if: steps.extract-pipeline.outputs.pipeline_id != '' | ||
| env: | ||
| PIPELINE_ID: ${{ steps.extract-pipeline.outputs.pipeline_id }} | ||
| run: | | ||
| curl --request POST \ | ||
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/$PIPELINE_ID/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline $PIPELINE_ID" |
| runs-on: ubuntu-24.04 | ||
| # Only trigger GitLab CI when: | ||
| # 1. The triggering workflow succeeded | ||
| # 2. It was triggered by a pull request | ||
| # 3. PR author is from the repository OR is an org member/collaborator | ||
| if: | | ||
| github.event.workflow_run.conclusion == 'success' && | ||
| github.event.workflow_run.event == 'pull_request' && | ||
| github.event.workflow_run.pull_requests[0] != null && ( | ||
| github.event.workflow_run.pull_requests[0].head.repo.full_name == github.repository || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'MEMBER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'OWNER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'COLLABORATOR' | ||
| ) | ||
| outputs: | ||
| json: ${{ steps.trigger.outputs.json }} | ||
| steps: | ||
| - name: Get PR details | ||
| id: pr | ||
| run: | | ||
| echo "number=${{ github.event.workflow_run.pull_requests[0].number }}" >> $GITHUB_OUTPUT | ||
| echo "head_sha=${{ github.event.workflow_run.pull_requests[0].head.sha }}" >> $GITHUB_OUTPUT | ||
| echo "head_ref=${{ github.event.workflow_run.pull_requests[0].head.ref }}" >> $GITHUB_OUTPUT | ||
| echo "title=${{ github.event.workflow_run.pull_requests[0].title }}" >> $GITHUB_OUTPUT | ||
| echo "repo=${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }}" >> $GITHUB_OUTPUT | ||
| echo "author=${{ github.event.workflow_run.pull_requests[0].user.login }}" >> $GITHUB_OUTPUT | ||
| echo "author_association=${{ github.event.workflow_run.pull_requests[0].author_association }}" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Log PR source and authorization | ||
| run: | | ||
| echo "PR #${{ steps.pr.outputs.number }} from: ${{ steps.pr.outputs.repo }}" | ||
| echo "Author: ${{ steps.pr.outputs.author }} (association: ${{ steps.pr.outputs.author_association }})" | ||
| if [ "${{ steps.pr.outputs.repo }}" != "${{ github.repository }}" ]; then | ||
| echo "::notice::PR from fork by trusted contributor (organization member/collaborator)" | ||
| fi | ||
|
|
||
| - uses: eic/trigger-gitlab-ci@v3 | ||
| id: trigger | ||
| with: | ||
| url: https://eicweb.phy.anl.gov | ||
| project_id: 290 | ||
| token: ${{ secrets.EICWEB_CONTAINER_TRIGGER }} | ||
| ref_name: master | ||
| variables: | | ||
| GITHUB_REPOSITORY=${{ github.repository }} | ||
| GITHUB_SHA=${{ steps.pr.outputs.head_sha }} | ||
| GITHUB_PR=${{ steps.pr.outputs.number }} | ||
| EICRECON_VERSION="${{ steps.pr.outputs.head_ref }}" | ||
| PIPELINE_NAME=${{ github.repository }}: ${{ steps.pr.outputs.title }} | ||
|
|
||
| - name: Post a GitHub CI status | ||
| run: | | ||
| gh api \ | ||
| --method POST \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| /repos/${{ github.repository }}/statuses/${{ steps.pr.outputs.head_sha }} \ | ||
| -f state="pending" \ | ||
| -f target_url="${{ steps.trigger.outputs.web_url }}" \ | ||
| -f description="Triggered... $(TZ=America/New_York date)" \ | ||
| -f context="eicweb/eic_container" | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Save GitLab pipeline ID for cancellation | ||
| run: | | ||
| mkdir -p gitlab-pipeline-info | ||
| echo "${{ steps.trigger.outputs.json }}" > gitlab-pipeline-info/pipeline-info.json | ||
| echo "GitLab pipeline ID saved for potential cancellation" | ||
|
|
||
| - name: Upload GitLab pipeline info as artifact | ||
| uses: actions/upload-artifact@v5 | ||
| with: | ||
| name: gitlab-pipeline-${{ github.event.workflow_run.id }} | ||
| path: gitlab-pipeline-info/ | ||
| retention-days: 1 |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix this problem, add an explicit permissions block to the workflow to limit what the automatically provided GITHUB_TOKEN can do. The permissions block should be placed either at the root of the workflow or for each job requiring specialized access. Here, the main job uses gh api to post commit statuses and uploads artifacts; it does not appear to need broad write access, so we should grant only what is strictly required. At minimum, the job needs statuses: write to post status checks. If you want every job in the workflow to inherit the same permissions and don't foresee other jobs that need broader access, put the permissions block at the root level. Otherwise, add it under the trigger-container job.
It is a best practice to start with the least privileges necessary. Based on the workflow, a root-level permissions block with statuses: write (and optionally, contents: read if needed for artifact upload etc) should suffice. Place the block just after the workflow name and on sections.
-
Copy modified lines R8-R11
| @@ -5,6 +5,10 @@ | ||
| workflows: ["Build against eic-shell"] | ||
| types: [completed] | ||
|
|
||
| permissions: | ||
| statuses: write | ||
| contents: read | ||
|
|
||
| jobs: | ||
| trigger-container: | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors GitLab CI triggering to enable secure execution of CI pipelines for pull requests from organization members' forks. The refactoring splits the triggering logic from the main workflow into separate workflows that use the workflow_run event, allowing them to run in the base repository context with access to secrets while maintaining security.
Key changes:
- Created
workflow_run-based trigger and cancellation workflows that run in base repo context with secret access - Implemented author association checks (MEMBER/OWNER/COLLABORATOR) to control which fork PRs can trigger CI
- Introduced artifact-based pipeline tracking to enable cancellation of the correct GitLab pipeline
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
.github/workflows/trigger-gitlab-ci.yml |
New workflow that triggers GitLab CI via workflow_run event when the main workflow succeeds, with security checks for PR author association |
.github/workflows/cancel-gitlab-ci.yml |
New workflow that cancels GitLab pipelines via workflow_run event when the main workflow fails or is cancelled, using artifacts to identify the correct pipeline |
.github/workflows/linux-eic-shell.yml |
Removed inline trigger-container and cancel-container jobs that have been replaced by the separate workflows |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: | | ||
| (github.event.workflow_run.conclusion == 'failure' || | ||
| github.event.workflow_run.conclusion == 'cancelled') && | ||
| github.event.workflow_run.event == 'pull_request' && | ||
| github.event.workflow_run.pull_requests[0] != null && ( | ||
| github.event.workflow_run.pull_requests[0].head.repo.full_name == github.repository || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'MEMBER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'OWNER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'COLLABORATOR' | ||
| ) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the trigger workflow, consider explicitly excluding dependabot to maintain consistency with the old workflow behavior:
if: |
(github.event.workflow_run.conclusion == 'failure' ||
github.event.workflow_run.conclusion == 'cancelled') &&
github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.actor != 'dependabot[bot]' &&
github.event.workflow_run.pull_requests[0] != null && (
...
)| curl --request POST \ | ||
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding error handling to the curl command to verify the cancellation was successful. For example:
run: |
response=$(curl --request POST --write-out "\n%{http_code}" --silent \
--header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \
"https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel")
http_code=$(echo "$response" | tail -n1)
if [ "$http_code" -ge 200 ] && [ "$http_code" -lt 300 ]; then
echo "::notice::Successfully cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}"
else
echo "::warning::Failed to cancel GitLab pipeline (HTTP $http_code)"
exit 1
fi| curl --request POST \ | |
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | |
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | |
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | |
| response=$(curl --request POST --write-out "\n%{http_code}" --silent \ | |
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | |
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel") | |
| http_code=$(echo "$response" | tail -n1) | |
| if [ "$http_code" -ge 200 ] && [ "$http_code" -lt 300 ]; then | |
| echo "::notice::Successfully cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | |
| else | |
| echo "::warning::Failed to cancel GitLab pipeline (HTTP $http_code)" | |
| exit 1 | |
| fi |
| echo "number=${{ github.event.workflow_run.pull_requests[0].number }}" >> $GITHUB_OUTPUT | ||
| echo "head_sha=${{ github.event.workflow_run.pull_requests[0].head.sha }}" >> $GITHUB_OUTPUT | ||
| echo "repo=${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }}" >> $GITHUB_OUTPUT | ||
| echo "author=${{ github.event.workflow_run.pull_requests[0].user.login }}" >> $GITHUB_OUTPUT |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the trigger workflow, user-controlled values from the PR (number, sha, repo, author) are being directly interpolated into shell commands without proper escaping. Consider using environment variables to safely pass these values:
run: |
echo "number=$NUMBER" >> $GITHUB_OUTPUT
env:
NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }}| echo "number=${{ github.event.workflow_run.pull_requests[0].number }}" >> $GITHUB_OUTPUT | |
| echo "head_sha=${{ github.event.workflow_run.pull_requests[0].head.sha }}" >> $GITHUB_OUTPUT | |
| echo "repo=${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }}" >> $GITHUB_OUTPUT | |
| echo "author=${{ github.event.workflow_run.pull_requests[0].user.login }}" >> $GITHUB_OUTPUT | |
| echo "number=$NUMBER" >> $GITHUB_OUTPUT | |
| echo "head_sha=$HEAD_SHA" >> $GITHUB_OUTPUT | |
| echo "repo=$REPO" >> $GITHUB_OUTPUT | |
| echo "author=$AUTHOR" >> $GITHUB_OUTPUT | |
| env: | |
| NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }} | |
| HEAD_SHA: ${{ github.event.workflow_run.pull_requests[0].head.sha }} | |
| REPO: ${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }} | |
| AUTHOR: ${{ github.event.workflow_run.pull_requests[0].user.login }} |
| name: gitlab-pipeline-${{ github.event.workflow_run.id }} | ||
| path: gitlab-pipeline-info/ | ||
| run_id: ${{ github.event.workflow_run.id }} | ||
| # Allow searching in other workflows since trigger-gitlab-ci created the artifact |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The comment states "Allow searching in other workflows" but should clarify that it's searching in other workflow runs, not other workflow types. Consider updating to: "Allow searching in other workflow runs since trigger-gitlab-ci created the artifact in a separate workflow_run"
| # Allow searching in other workflows since trigger-gitlab-ci created the artifact | |
| # Allow searching in other workflow runs since trigger-gitlab-ci created the artifact in a separate workflow_run |
| fi | ||
| else | ||
| echo "Pipeline info file not found in artifact" | ||
| exit 1 |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to line 63, this exits with code 1 when the file is not found, but graceful handling seems more appropriate given the continue-on-error: true on the download step. Consider using exit 0 or adding continue-on-error: true to this step.
| exit 1 | |
| exit 0 |
| echo "number=${{ github.event.workflow_run.pull_requests[0].number }}" >> $GITHUB_OUTPUT | ||
| echo "head_sha=${{ github.event.workflow_run.pull_requests[0].head.sha }}" >> $GITHUB_OUTPUT | ||
| echo "head_ref=${{ github.event.workflow_run.pull_requests[0].head.ref }}" >> $GITHUB_OUTPUT | ||
| echo "title=${{ github.event.workflow_run.pull_requests[0].title }}" >> $GITHUB_OUTPUT | ||
| echo "repo=${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }}" >> $GITHUB_OUTPUT | ||
| echo "author=${{ github.event.workflow_run.pull_requests[0].user.login }}" >> $GITHUB_OUTPUT | ||
| echo "author_association=${{ github.event.workflow_run.pull_requests[0].author_association }}" >> $GITHUB_OUTPUT |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR title, branch name, and other user-controlled values are being directly interpolated into shell commands and GitLab variables without proper escaping. If a PR title contains special characters like quotes, backticks, or newlines, this could lead to command injection or unexpected behavior.
Consider using environment variables or proper quoting:
run: |
echo "number=$NUMBER" >> $GITHUB_OUTPUT
env:
NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }}Or at minimum, ensure values are properly quoted when used in subsequent steps.
| echo "number=${{ github.event.workflow_run.pull_requests[0].number }}" >> $GITHUB_OUTPUT | |
| echo "head_sha=${{ github.event.workflow_run.pull_requests[0].head.sha }}" >> $GITHUB_OUTPUT | |
| echo "head_ref=${{ github.event.workflow_run.pull_requests[0].head.ref }}" >> $GITHUB_OUTPUT | |
| echo "title=${{ github.event.workflow_run.pull_requests[0].title }}" >> $GITHUB_OUTPUT | |
| echo "repo=${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }}" >> $GITHUB_OUTPUT | |
| echo "author=${{ github.event.workflow_run.pull_requests[0].user.login }}" >> $GITHUB_OUTPUT | |
| echo "author_association=${{ github.event.workflow_run.pull_requests[0].author_association }}" >> $GITHUB_OUTPUT | |
| printf "number=%s\n" "${{ github.event.workflow_run.pull_requests[0].number }}" >> $GITHUB_OUTPUT | |
| printf "head_sha=%s\n" "${{ github.event.workflow_run.pull_requests[0].head.sha }}" >> $GITHUB_OUTPUT | |
| printf "head_ref=%s\n" "${{ github.event.workflow_run.pull_requests[0].head.ref }}" >> $GITHUB_OUTPUT | |
| printf "title=%s\n" "${{ github.event.workflow_run.pull_requests[0].title }}" >> $GITHUB_OUTPUT | |
| printf "repo=%s\n" "${{ github.event.workflow_run.pull_requests[0].head.repo.full_name }}" >> $GITHUB_OUTPUT | |
| printf "author=%s\n" "${{ github.event.workflow_run.pull_requests[0].user.login }}" >> $GITHUB_OUTPUT | |
| printf "author_association=%s\n" "${{ github.event.workflow_run.pull_requests[0].author_association }}" >> $GITHUB_OUTPUT |
| variables: | | ||
| GITHUB_REPOSITORY=${{ github.repository }} | ||
| GITHUB_SHA=${{ steps.pr.outputs.head_sha }} | ||
| GITHUB_PR=${{ steps.pr.outputs.number }} | ||
| EICRECON_VERSION="${{ steps.pr.outputs.head_ref }}" | ||
| PIPELINE_NAME=${{ github.repository }}: ${{ steps.pr.outputs.title }} |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User-controlled values (head_ref and title) from the PR are being passed to GitLab CI variables without validation. The PR title and branch name could contain special characters, newlines, or other content that could cause issues in the GitLab pipeline or be used maliciously.
While the main security model correctly runs this in the base repo context, consider sanitizing or validating these values before passing them to external systems, or ensure the GitLab pipeline properly handles untrusted input.
| if: | | ||
| github.event.workflow_run.conclusion == 'success' && | ||
| github.event.workflow_run.event == 'pull_request' && | ||
| github.event.workflow_run.pull_requests[0] != null && ( | ||
| github.event.workflow_run.pull_requests[0].head.repo.full_name == github.repository || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'MEMBER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'OWNER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'COLLABORATOR' | ||
| ) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old workflow explicitly excluded dependabot[bot] from triggering GitLab CI. While dependabot PRs come from forks and might not have MEMBER association, if dependabot is granted any level of association, it could trigger GitLab CI with secrets. Consider adding an explicit exclusion for dependabot:
if: |
github.event.workflow_run.conclusion == 'success' &&
github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.actor != 'dependabot[bot]' &&
github.event.workflow_run.pull_requests[0] != null && (
...
)Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: | | ||
| (github.event.workflow_run.conclusion == 'failure' || | ||
| github.event.workflow_run.conclusion == 'cancelled') && | ||
| github.event.workflow_run.event == 'pull_request' && | ||
| github.event.workflow_run.pull_requests[0] != null && ( | ||
| github.event.workflow_run.pull_requests[0].head.repo.full_name == github.repository || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'MEMBER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'OWNER' || | ||
| github.event.workflow_run.pull_requests[0].author_association == 'COLLABORATOR' | ||
| ) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic issue: This workflow attempts to cancel a GitLab pipeline when the main workflow fails or is cancelled, but it retrieves the pipeline ID from an artifact that only exists if the main workflow succeeded (trigger-gitlab-ci only runs on success). If the workflow fails before or during the build, no artifact will exist and cancellation will silently do nothing.
Consider either:
- Having trigger-gitlab-ci run earlier (on workflow start instead of completion), or
- Storing the pipeline ID in a different way that persists across workflow re-runs, or
- Querying GitLab API to find the pipeline by commit SHA if the artifact is missing.
| curl --request POST \ | ||
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | ||
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | ||
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The curl command should include error handling (e.g., --fail-with-body or checking the exit code) to properly report if the cancellation request fails. Without this, a failed cancellation attempt will still show success in the workflow logs, making debugging difficult.
| curl --request POST \ | |
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | |
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | |
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | |
| set -e | |
| curl --fail-with-body --request POST \ | |
| --header "PRIVATE-TOKEN: ${{ secrets.EICWEB_CONTAINER_CANCEL }}" \ | |
| "https://eicweb.phy.anl.gov/api/v4/projects/290/pipelines/${{ steps.extract-pipeline.outputs.pipeline_id }}/cancel" | |
| if [ $? -eq 0 ]; then | |
| echo "::notice::Cancelled GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | |
| else | |
| echo "::error::Failed to cancel GitLab pipeline ${{ steps.extract-pipeline.outputs.pipeline_id }}" | |
| exit 1 | |
| fi |
| @@ -0,0 +1,92 @@ | |||
| name: Trigger GitLab CI | |||
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing explicit permissions declaration. Following the principle of least privilege, this workflow should explicitly declare the permissions it needs. Based on the workflow's actions, it needs:
statuses: write(for posting GitHub CI status)actions: read(for accessing workflow_run data)contents: read(for repository data)
Add a permissions: block at the workflow level to explicitly grant only the required permissions, similar to cancel-gitlab-ci.yml.
| name: Trigger GitLab CI | |
| name: Trigger GitLab CI | |
| permissions: | |
| statuses: write | |
| actions: read | |
| contents: read |
| id: trigger | ||
| with: | ||
| url: https://eicweb.phy.anl.gov | ||
| project_id: 290 |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential YAML injection risk: The PR title is directly interpolated into the variables block without escaping. If a PR title contains special characters like colons, newlines, or YAML control characters, it could break the YAML syntax passed to the GitLab CI trigger action or be interpreted as additional YAML structure. Consider sanitizing the title or using quotes around the value.
| run: | | ||
| gh api \ | ||
| --method POST \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GitHub status posting step should include continue-on-error: true to prevent workflow failure if the status API call fails. This ensures that a GitLab pipeline can still be triggered even if posting the status fails, and prevents blocking the workflow on non-critical operations.
| run: | | |
| gh api \ | |
| --method POST \ | |
| continue-on-error: true | |
| run: | | |
| gh api \ | |
| --method POST \ |
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Save GitLab pipeline ID for cancellation | ||
| run: | | ||
| mkdir -p gitlab-pipeline-info | ||
| echo "${{ steps.trigger.outputs.json }}" > gitlab-pipeline-info/pipeline-info.json |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The artifact upload step should include continue-on-error: true to prevent workflow failure if the upload fails. While this artifact is useful for cancellation, the inability to upload it shouldn't fail the entire workflow since the GitLab pipeline has already been triggered successfully.
Briefly, what does this PR introduce?
This PR splits GitLab CI triggering into separate workflows using
workflow_run:trigger-gitlab-ci.yml: triggers when main workflow is createdcancel-gitlab-ci.yml: cancels pipeline when main workflow failsSecurity model:
workflow_runruns on default branch (never executes PR code)Pipeline cancellation uses artifacts:
workflow_run.idas keyWhat kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No.
Does this PR change default behavior?
No.