Skip to content

[2.29.x] Use pull_request_target to build PRs from forks#6979

Open
alexabird wants to merge 1 commit into2.29.xfrom
forks
Open

[2.29.x] Use pull_request_target to build PRs from forks#6979
alexabird wants to merge 1 commit into2.29.xfrom
forks

Conversation

@alexabird
Copy link
Member

@alexabird alexabird commented Mar 4, 2026

What does this PR do?

PRs from forks failed CI because the pull_request action event builds on the forked repository and does not have access to this repo's secret in order to pull the github packages during the build.

⚠️ Security Warning

Using pull_request_target with PRs from untrusted external contributors is inherently risky and requires careful implementation to prevent secret exfiltration or unauthorized repository modifications. The workflow runs in the security context of the base branch, so an attacker can modify the PR's code to print or otherwise expose your secrets if the workflow simply checks out and runs the untrusted code from the PR.

I think this is fairly safe though since we require approval for CI to run for outside contributors - just want to confirm this - or if we can/should require approval for CI run? Noting also that the READ_PACKAGES secret only has the packages:read permissions.

Any background context you want to provide?

PR from fork with failing build: #6972
tested with a PR from DDF into my forked repo: alexabird#1

Review Comment Legend:

  • ✏️ (Pencil) This comment is a nitpick or style suggestion, no action required for approval. This comment should provide a suggestion either as an in line code snippet or a gist.
  • ❓ (Question Mark) This comment is to gain a clearer understanding of design or code choices, clarification is required but action may not be necessary for approval.
  • ❗ (Exclamation Mark) This comment is critical and requires clarification or action before approval.

@alexabird alexabird requested a review from jaymcnallie March 4, 2026 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant