As an external contributor of GCNotify,
I want to submit code / documentation fixes to the code base,
So that I can improve the product and fix issues by my own.
As a developer of GCNotify,
I want PRs coming from external contributors to be merged through an easier yet formal process,
So that I can review and merge code into the code base in a safe manner,
And without having to create a new PR with manual changes of my own with proposed code.
WHY are we building?
To promote and streamline contributions from non-CDS members.
WHAT are we building?
Make PRs from external contributors easy to merge in. At the moment, we cannot merge these for security purposes as the CI should not automatically run code that could contain malicious contributions once a PR is opened.
On the other hand though, there is no process to merge a PR once it has deemed safe. We need to get past this limitations. Ideally, keeping the original PR would be good to identify the external authors as well.
The process would be as follows:
External contributor opens a new PR.
Static code analysis runs on the suggested code, but no regular CI actions should run such as building the application.
The GCNotify team assigns 2 developers to review the PR and evaluate its safety and value. Possibly tag someone from the security team as well, depending on the nature and complexity of the changes (i.e. a regular documentation changes would not require security).
A potential back and forth for code changes request occur between the GCNotify reviewers and external contributor.
If deemed safe and valuable, then the GCNotify reviewers tag the PR with a public-safeprotected label.
Once the public-safe tag is applied, the usual CI/CD Github actions are ran, such as the test action.
Any new code changes applied to the branch removes the public-safe tag.
A final proper review is conducted by 2 GCNotify developers, different from the previous review phase.
Once approved and with the public-safe tag still on (to guarantee there were no new code changes by the external contributor in the meantime), we are good to merge in staging.
VALUE created by our solution
We are more receptive to external contributions which can improve our software offering.
Acceptance Criteria
[ ] A security stamp approval is implemented on our external contribution PR (via public-safe), which prevents execution of the regular code pipeline.
[ ] A written document explain the overall process, probably proposed as an ADR and reviewed by the security team.
[ ] The solution is implemented into the Github actions of the API repository as a first proof of concept prior to other repositories.
QA Steps
[ ] Use an external Github user ID, non-familiar to the CDS organisation, to test a code contribution.
[ ] Organise a bug bash to break this feature as much as we can.
The ability to run the CI/CD pipeline would run on review approval. After the approval, is it possible for the external contributors to still commit?
It might be desired to apply an approval removal on new commits for the reason that a contributor could still commit after an approved review. We should thoroughly test.
It's possible that the ability to run CI/CD on external contributors PR is disabled at the CDS organization level. Pat tells me that at org level, approval for outside collaborators should be enabled for running workflows.
We need to test and bug bash different scenarios using this solution.
Description
As an external contributor of GCNotify, I want to submit code / documentation fixes to the code base, So that I can improve the product and fix issues by my own.
As a developer of GCNotify, I want PRs coming from external contributors to be merged through an easier yet formal process, So that I can review and merge code into the code base in a safe manner, And without having to create a new PR with manual changes of my own with proposed code.
WHY are we building?
To promote and streamline contributions from non-CDS members.
WHAT are we building?
Make PRs from external contributors easy to merge in. At the moment, we cannot merge these for security purposes as the CI should not automatically run code that could contain malicious contributions once a PR is opened.
On the other hand though, there is no process to merge a PR once it has deemed safe. We need to get past this limitations. Ideally, keeping the original PR would be good to identify the external authors as well.
The process would be as follows:
public-safeprotected label.public-safetag is applied, the usual CI/CD Github actions are ran, such as the test action.public-safetag.public-safetag still on (to guarantee there were no new code changes by the external contributor in the meantime), we are good to merge in staging.VALUE created by our solution
We are more receptive to external contributions which can improve our software offering.
Acceptance Criteria
public-safe), which prevents execution of the regular code pipeline.QA Steps
Additional information
Alternatives
It's possible we could make use of Github default's mechanism for external contributors, but it might open up some security concerns. A few thoughts:
Resources