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

Actions: Securely run test workflows on forks (PROTOTYPE - do not merge) #1334

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

kyleecodes
Copy link
Member

@kyleecodes kyleecodes commented Feb 17, 2025

Resolves #enter-issue-number

N/A

What changes did you make and why did you make them?

Previously, the build-and-test workflow was triggered by the pull_request event trigger, but this does not grant secrets access to forks, which is necessary to run unit tests. The inverse is true for pull_request_target, except that it's ineffective and misleading, as it tests our develop branch without an explicit code ref checkout. Explicit checkouts with pull_request_target are strongly discouraged and goes against it's use-cases.
Given the security steps we've already taken, including the approval step and testing secrets auditing, we decided to keep the workflow and grant access to secrets on forks. For increased security, I implemented an auto-labeler to automatically label any forked PRs making changes to sensitive files (such as scripts, package.json, workflows, etc) to circumvent human error. This PR fixes our build and test workflows for forks, effectively prevents prebuild jobs without proper permissions, and adds a layer of protection against human error in PR reviews.

To-Do:

  • Test & audit in GH org environment
  • Implement correct dependency flow with workflow_run
  • Exclude i18n and other non-config .json files in labeler config
  • Add label for no sensitive files found

Actions Changelog:

  • labeler.yml: config for the labeler workflow, determines sensitive files.
  • community-label-forked-prs: labels forked pull requests with 'needs-file-review` if sensitive files detected.
  • build-and-test-prs: updated to exit if the triggering PR is a fork, to avoid the dependency installs on forks.
  • build-and-test-forks runs workflow on forks only, exiting before build if sensitive file label is detected. This can be manually removed by staff after reviewing the files.
  • Pins third-party commits to SHA
  • Auto-assign dependabot PRs
  • More succinct pull request template

Resources:

Did you run tests? Share screenshot of results:

How did you find us? (GitHub, Google search, social media, etc.):

Copy link

vercel bot commented Feb 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bloom-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 17, 2025 9:08pm

@kyleecodes kyleecodes changed the title Actions: Secure workflow run on forks + dependabot auto-assign Actions: Secure test workflows on forks Feb 17, 2025
@kyleecodes kyleecodes changed the title Actions: Secure test workflows on forks Actions: Securely run test workflows on forks Feb 17, 2025
@kyleecodes kyleecodes changed the title Actions: Securely run test workflows on forks Actions: Securely run test workflows on forks (PROTOTYPE - do not merge) Feb 20, 2025
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.

1 participant