Diff comment does not run off PRs from fork

[matentzn]

Suggestion: Perhaps update the ancient GH_ACTION token in https://github.com/obophenotype/uberon/settings/secrets/actions with improved permissions?

[gouttegd]

Background

A pull request on a repository is basically nothing more than a request that a given branch (hereafter called the source branch) be merged into another branch (hereafter called the target branch). Typically, someone has done some work in the source branch, and requests that their work be merged in the project’s master (or main) branch.

The source branch does not need to belong to the same repository as the target branch. If it does belong to the same repo, we will call that an internal PR; otherwise, it’s an external PR.

Why do some people create external PRs instead of internal PRs? Because that may be the only way for them to contribute to a project. You need write access to a repository in order to create a branch in said repository. If you don’t have write access to the repository of the project you want to contribute to, your only solution is to fork the repository (create your own copy of it), push your changes to a branch in your forked copy (you have the right to do that, since it’s your fork), and then create a (external) pull request on the original repository.

Without the ability to create external PRs, the only way to contribute to a project would be to obtain write access first (or to send patches by email, but seemingly nobody wants to do that anymore). This is fine for a closed model of development, but not for any kind of free/libre software (or science) development.

The point is that we don’t want to discourage external PRs, and we’d want all of the services we are currently doing through GitHub Actions (like CI checking and diff’ing) to perform equally well on both internal and external PRs.

The problem

Simply put: GitHub Action workflows that are triggered by external PRs do not have any write access to the repository. This is for security reason: the PR is originating from a third-party which could be malicious, and therefore cannot be trusted with writing access.

So workflows triggered by external PRs cannot do anything that requires write access, such as posting a comment on a ticket (which is exactly what we do want to do in the “gogoeditdiff” workflow).

It is not a configuration issue: There is no way to configure the workflow to grant write access when the workflow is triggered by an external PR, period. The only case where it is possible to do such a thing is for private repositories: For private repositories, there is an option called Send write tokens to workflows from pull requests which does grant write access to workflows triggered from external PRs. But that option is not available for public repositories. (The reason for allowing that for private repositories only is that, with a private repository, you control who is able to fork the repository, so any external PR can still only come from someone you once allowed to fork the repository, so someone you can decide to trust.)

The solution

To work around the fact that a workflow triggered from an external PR cannot post a comment on the repository, the only foolproof solution is to break down the workflow in two separate workflows:

A) a “main” workflow that is triggered when a PR (internal or external) is created, does whatever it has to do with the PR (e.g. running the test suite or creating a diff report), and then uploads the results as workflow artifacts (the artifacts are not stored within the repo, so perhaps counter-intuitively uploading an artifact does not require write access);

B) a “secondary” workflow that is triggered upon completion of the “main” workflow, and whose only task is to fetch the artifact(s) created by the “main” workflow and post them as comments on the PR.

The secondary workflow can have write access since it is only triggered internally and has no relation whatsoever with the external repo from which the PR was created.

See this blog post for an example of such a two-steps workflow.

[github-actions]

This issue has not seen any activity in the past 6 months; it will be closed automatically one year from now if no action is taken.

[matentzn]

In the Uberon/CL/RO tech team meeting we decided that this issue has higher complexity than is worth it (both on maintenance and implementation side). Feel free to reopen if becomes a priority.