mitodl / ocw-hugo-themes

A Hugo theme for building OCW websites
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

CI workflow for external contributions #1277

Open HussainTaj-arbisoft opened 1 year ago

HussainTaj-arbisoft commented 1 year ago

Description/Context

Our current CI workflow will not work with external contributions (PRs) because it uses secrets. Secrets and variables are not available to workflow runs that run on PRs from forks.

Example: https://github.com/mitodl/ocw-hugo-themes/pull/1274

It is possible to configure GitHub actions to work with external PRs, however, there are some security implications.

https://github.com/mitodl/ocw-hugo-themes/blob/cfb4ba27b4e7e911f2468f40df13e5d49945f7a8/.github/workflows/ci.yml#L31-L34

Plan/Design

The following are some ideas we can try.

  1. The secrets that are being used do not seem like secrets to me. To my knowledge, all of these URLs are publically discoverable. Adding them in code is one option, but I can understand why that would be undesirable.

    • Pros: Less changes required
    • Cons: Not the best quality of code
  2. An alternative is to separate e2e tests into a different workflow via workflow_run trigger.

    • Pros: Does not reveal secrets
    • Cons: Does not run contributed e2e tests
  3. Not recommended. Use pull_request_target trigger. Add manual confirmation steps, like ensuring the workflow runs with a certain tag on a PR. This ensures that someone manually verifies the code before letting the workflow run. However, this article explains that this could also be exploited.

    • Pros: Runs contributed tests
    • Cons: Reveals secrets
gumaerc commented 1 year ago

@HussainTaj-arbisoft The idea behind putting these into secrets was for the values to be easy to update through the Github UI without having to make a code change. I think it's proven at this point that these values rarely change, if ever. You are correct that they are not secrets and can likely just be hard coded into the ci.yml definition as in your first suggestion.