Skip to content
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: update workflow to pull_request_target #7

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Conversation

fedorov
Copy link
Member

@fedorov fedorov commented Mar 22, 2024

Current pull_request does not allow PRs from forks to have access to the repo secrets.

See details in https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

Current pull_request does not allow PRs from forks to have access to the repo
secrets.

See details in https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
@jcfr
Copy link
Collaborator

jcfr commented Mar 22, 2024

Usually, access to secrets is specific to the deployment jobs and associated workflows, for this reason there is no reason to allow access to secrets from pull request coming from fork.

In this case, since the "regular" jobs are expected to access the secret required to run the query, it may indeed be helpful to allow pull request to access the corresponding secret.

Alternatively, we could require maintainer to create branches and publish them in this repo.

As outlined in the post1, it will be critical to careful review pull request from external contributor we do not know before clicking the Approve and Run when their first pull request is submitted.

Footnotes

  1. https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

@jcfr jcfr deleted the update-gha-target branch March 22, 2024 16:10
@jcfr
Copy link
Collaborator

jcfr commented Mar 23, 2024

After reading more carefully the documentation, this should be reverted as it prevents workflow triggered in pull request from effectively testing the code being proposed.

See https://stackoverflow.com/questions/74957218/what-is-the-difference-between-pull-request-and-pull-request-target-event-in-git

@jcfr
Copy link
Collaborator

jcfr commented Mar 23, 2024

Few paths forward here:

  1. If we are okay with having pull-request accessing the "secret" (aka SERVICE_ACCOUNT_KEY), it would be easier to simply add it as a regular variable. This would avoid applying workaround requiring to explicit checkout the actual branch associated with the pull request. See INSECURE. Provided as an example only. example in the post1.

  2. Revert this pull request, skip the part of the workflow that do not have access to the secret reporting a meaningful message indicating that they need to be added as collaborator. Indeed pull request from collaborator (with "write" access) will have access2 to secret.

  3. Implement a more sophisticated set of workflows with a first one able to access the secret to generate the index-data file and uploading it as an artifact to then ensure a second workflow effectively test the pull request code by using the file generated.

Moving forward with (2) seems like the most reasonable solution here.

Footnotes

  1. https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

  2. https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions?tool=webui#accessing-your-secrets

vkt1414 pushed a commit that referenced this pull request Apr 9, 2024
Current pull_request does not allow PRs from forks to have access to the repo
secrets.

See details in https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
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.

None yet

2 participants