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

CI workflow for external contributions #1277

Open
HussainTaj-arbisoft opened this issue Nov 7, 2023 · 1 comment
Open

CI workflow for external contributions #1277

HussainTaj-arbisoft opened this issue Nov 7, 2023 · 1 comment

Comments

@HussainTaj-arbisoft
Copy link
Contributor

HussainTaj-arbisoft commented Nov 7, 2023

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: #1274

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

OCW_STUDIO_BASE_URL: ${{ secrets.OCW_STUDIO_BASE_URL }}
SEARCH_API_URL: ${{ secrets.SEARCH_API_URL }}
STATIC_API_BASE_URL: ${{ secrets.STATIC_API_BASE_URL }}
RESOURCE_BASE_URL: ${{ secrets.RESOURCE_BASE_URL }}

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
Copy link
Collaborator

gumaerc commented Nov 7, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants