threshold-network / website

1 stars 10 forks source link

Make sure NuCypher team and DAO contributors can open deployable PRs #86

Closed mhluongo closed 10 months ago

mhluongo commented 1 year ago

The last time @mswilkison opened a PR, the build failed... and we're not sure why.

Let's fix it so anyone at the DAO can update the site!

michalinacienciala commented 1 year ago

The problem with failing checks there was likely caused by the fact that changes were pushed from the forked repo. And according to GH:

In order to protect public repositories for malicious users we run all pull request workflows raised from repository forks with a read-only token and no access to secrets. This makes common workflows like labeling or commenting on pull requests very difficult.

Source: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks

Possible fix to be considered: switching to the pull_request_target trigger event in the workflow.

nkuba commented 1 year ago

The reason could be a lack of permissions - @mswilkison is not a member of the @threshold-network/developers group. We need to fix that oversight!

The broader issue is handling external contributions. As @michalinacienciala mentioned the PR was created from a forked repository and GitHub has a protection against secrets leakage that could block execution here.

Unfortunately, execution logs are no longer available. We will need a deeper investigation to find the best solution. On top of my head, I see two solutions:

  1. Short-term: Authorized contributors should branch off the main branch in the threshold-network/splash-page repository (without forking the repo).
  2. Long-term: Use a combination of pull_request_target (nice finding @michalinacienciala!) and/or workflows approvals.
nkuba commented 1 year ago

However, instead of running against the workflow and code from the merge commit, the event runs against the workflow and code from the base of the pull request. -- source

If I understand correctly pull_request_target may not be a solution.

erdogan commented 10 months ago

NuCypher is able to contribute now, external contribution setup is to be planned