search

ni / semi-test-library-dotnet

Semiconductor Test Library
https://ni.github.io/semi-test-library-dotnet/
MIT License
3 stars 2 forks source link

Add support to securely run workflows from forked repositories #148

Closed Tanish-NI closed 1 week ago

Tanish-NI commented 3 weeks ago

What does this Pull Request accomplish?

The current GitHub workflow implementation is not capable of supporting contributions from outside contributors (externally forked repositories). However, this is unintended and outside contributions should be fully supported, as is outlined in CONTRIBUTING.md.

The Pre-Build and Pre-Test workflows on GitHub use Azure Token to call internal Azure Pipelines. This token is stored as a repository secret which cannot be accessed by PRs from forked repositories. When an external contributor raises a PR from their fork, these workflows will fail, blocking the necessary checks from running against their code.

Why should this Pull Request be merged?

Added 3 new workflows:-

  1. pr-build
  2. pr-test
  3. pr-intake

The idea is to separate the PR Intake workflow from the PR Build workflow, as suggested in the GitHub Security Lab Article using the workflow-run event along with pull_request. But as in this case workflow_run event will be triggered out of the context of the pull_request workflow to access the AzDo token, to communicate between these workflows, GitHub APIs are used.

pr-intake workflow will be triggered when a PR is raised and pr-build & pr-test workflow will be triggered when pr-intake workflow is approved manually (as using gh-action-testing environment) and starts running. pr-build and pr-test will call Azure pipelines. And pr-intake uses GitHub APIs to check status and conclusion of both runs.

What testing has been done?

Pending, because of the Note mentioned in workflow_run GitHub documentation, the pr-build and pr-test workflows need to be in main to trigger them via workflow_run event. Hence, in this case, testing will be done after the PR went in the main (default branch). image

Tanish-NI commented 2 weeks ago

@Mattjet27 I think we should not remove the existing Pre Build and Pre Test workflows right away in this PR. Because still, some more testing needs to be done using these new workflows after this PR gets in. Once we test this new way of calling Azure pipelines, after that it seems more promising to remove the existing workflows.