-
Notifications
You must be signed in to change notification settings - Fork 280
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
ci(repo): Use pull_request_target
event by triggering actor permissions
#4692
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: ceb4277 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
42e6729
to
8a03c9a
Compare
0808470
to
c8b34ed
Compare
5b621c6
to
86842b8
Compare
pull_request_target
event by checking user permissions
86842b8
to
dd34438
Compare
f8f0d1f
to
753f3fd
Compare
pull_request_target
event by checking user permissions pull_request_target
event by triggering actor permissions
753f3fd
to
e3795f7
Compare
.github/workflows/ci.yml
Outdated
@@ -2,13 +2,9 @@ name: CI | |||
|
|||
on: | |||
workflow_dispatch: | |||
inputs: | |||
run_integration_tests: |
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.
This wasn't being used at all, it was introduced in a previous PR with the attempt to continue using pull_request
event with workflow_run
34109cb
to
9b71e9c
Compare
9b71e9c
to
1ca9c63
Compare
1ca9c63
to
ceb4277
Compare
Description
#4702 needs to be merged first
This PR ensures that when a new untrusted PR is submitted, it verifies the triggering actor permissions, to proceed with the next steps that rely on repository secrets.
Context
Currently, CI steps that rely on secrets get skipped once it detects that it's an external fork. It's also using
pull_request
event which doesn't provide access to secrets. This is bringing a ton of friction to merge external contributions.Security considerations with
pull_request_target
Any automated processing of PRs from an external fork is potentially dangerous and such PRs should be treated like untrusted input. Malicious changes could be such as:
With the
pull_request
event, as per the documentation, with the exception ofGITHUB_TOKEN
, secrets are not passed to the runner when a workflow is triggered from a forked repository.On the other hand, workflows triggered via
pull_request_targe
have write permission to the target repository. They also have access to target repository secrets.pull_request_target
+ checking trigger actor permissionsGitHub Actions provides a triggering actor field that lets you know who ran (or re-ran) a workflow. This allows maintainers to check the changes manually, and proceed by re-running the jobs.
The workflow is:
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change