Skip to content

Revert pr.yaml trigger to pull_request (unblock CI) #80

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.

Sign up for GitHub

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

Closed
Chris-Wolfgang wants to merge 2 commits into from
Closed

Revert pr.yaml trigger to pull_request (unblock CI) #80

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1c743b
Revert pr.yaml trigger from pull_request_target to pull_request
Chris-Wolfgang Apr 19, 2026
e50f176
Restore security note about protected config file behavior
Chris-Wolfgang Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 6 additions & 25 deletions .github/workflows/pr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
# Stage 3: macOS tests (only if Stage 2 passes)
#
# SECURITY NOTE:
# - Uses pull_request_target to run workflow from the trusted main branch, not from the PR branch
# - This prevents malicious workflow YAML changes in untrusted PR branches from taking effect
# - All checkout steps use PR refs (refs/pull/*/head) to check out PR code from the base repo
# - Uses pull_request (not pull_request_target) to avoid the "pwn request" attack vector
# (pull_request_target runs from the trusted main branch with elevated permissions, and checking
# out untrusted PR code in that context can allow attackers to exfiltrate secrets or abuse write access)
# - After checkout, configuration files (.editorconfig, BannedSymbols.txt, etc.) are fetched from
# the main branch to prevent malicious PRs from disabling analyzers or bypassing code quality checks
Comment thread
Chris-Wolfgang marked this conversation as resolved.
# - If a PR changes any of these protected configuration files, CI explicitly fails with instructions
# for a maintainer to manually review and verify the changes before merging
# - persist-credentials: false prevents the checkout token from being written to git config for subsequent git commands
# (it does NOT, by itself, prevent steps from accessing github.token / GITHUB_TOKEN if you explicitly expose it)
# - PRs that change protected configuration files (for example .editorconfig or Directory.Build.*)
# intentionally fail validation; those config changes are not evaluated by CI in the untrusted PR context
# - Default GITHUB_TOKEN permissions are restricted to read-only repository contents to limit impact if exposed

name: PR Checks v3 (Gated)
Expand All @@ -24,7 +22,7 @@ env:
CODECOV_MINIMUM: 90

on:
pull_request_target: # Runs from the main branch, not from PR branch
pull_request:
branches:
- main
Comment on lines 24 to 27
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching the trigger to pull_request means the workflow definition is now sourced from the PR branch itself, so an untrusted PR can potentially modify this workflow to skip checks while still producing green check runs. If this repository needs to be hardened for external contributors, consider mitigating by running the validation workflow from the default branch (e.g., pull_request_target with tightly-scoped permissions and no secrets, or a reusable workflow invoked from a trusted workflow on main), and/or adding repo/ruleset protections for changes under .github/workflows/.

Copilot uses AI. Check for mistakes.

Expand All @@ -44,8 +42,6 @@ jobs:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/head
persist-credentials: false
fetch-depth: 0
Comment on lines 42 to 45
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the explicit ref: refs/pull/.../head changes actions/checkout to the default PR ref (typically the merge commit). If the intent is to keep validating the PR head commit (as before), set the checkout ref explicitly (e.g., to the PR head SHA). Also, removing persist-credentials: false causes the GITHUB_TOKEN credential to be written into the local git config by default; if you want to minimize token exposure to subsequent steps, consider restoring persist-credentials: false and updating any later git fetch operations to authenticate explicitly when needed.

Copilot uses AI. Check for mistakes.

- name: Run gitleaks
Expand All @@ -70,9 +66,6 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/head
persist-credentials: false

- name: Fetch trusted configuration files from main branch
run: |
Expand Down Expand Up @@ -194,9 +187,6 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/head
persist-credentials: false

- name: Fetch trusted configuration files from main branch
run: |
Expand Down Expand Up @@ -496,9 +486,6 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/head
persist-credentials: false

- name: Fetch trusted configuration files from main branch
shell: pwsh
Expand Down Expand Up @@ -735,9 +722,6 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/head
persist-credentials: false

- name: Fetch trusted configuration files from main branch
run: |
Expand Down Expand Up @@ -1044,9 +1028,6 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: refs/pull/${{ github.event.pull_request.number }}/head
persist-credentials: false

- name: Fetch trusted configuration files from main branch
run: |
Expand Down