fix: preview-web can't be run on forks

[pfeerick]

GitHub repo SECRETS are not exposed to forks, thus the preview-web step can't be run on pull requests from forks.

It may be possible to use a different event trigger... pull_request_target ... in order to get around this, but this then also changes the context of the base of the pull request, from it being the merge commit, to that of the base of the pull request, and am not sure if this will have unexpected consequences, hence this more immediate quick fix.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 41.46%. Comparing base (32b0cd3) to head (6bb4b3d). Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #84   +/-   ##
=======================================
  Coverage   41.46%   41.46%           
=======================================
  Files         103      103           
  Lines       12059    12059           
  Branches      525      525           
=======================================
  Hits         5000     5000           
  Misses       7050     7050           
  Partials        9        9           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[freshollie]

@pfeerick this makes sense, you could also try implement something like this https://michaelheap.com/access-secrets-from-forks/

[pfeerick]

Oh... thanks for that ... will look at that tomorrow :)

[pfeerick]

Ok, either something has changed in github, or that method only works for checkout... so it looks like the only real way to do this is how github themselves recommended it... by using a split workflow... one being the "insecure", and the second the "secure" one. Thankfully the current structure of the CI build seems to be suited to being split right down the middle where it's needed, so it shouldn't be that hard to simply split it in two.

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

[pfeerick]

Ok, I'm done for today :face_with_head_bandage: Both stage 1 and stage 2 appear to be working as intended.

stage 1 - test-and-compile - https://github.com/pfeerick/buddy/actions/runs/9591978099 stage 2 - build-app-and-preview - https://github.com/pfeerick/buddy/actions/runs/9592007656 stage 3 - release - https://github.com/pfeerick/buddy/actions/runs/9592184521

stage 2 was expected to fail where it did as while I gave it a valid cloudflare API and re-ran just that failed step, I don't have a mirror of the buddy project setup, so it merely progressed further than the first run.

stage 3 appears to have correctly skipped due to the failure of the prior stage.

Now sort of wondering if I should split it down a bit further... i.e. five workflows ... test, compile, app, web, release ... or maybe rename... test, build, release? :thinking:

[freshollie]

@pfeerick although this seems to work, I think a simpler option is to manually checkout the branch head SHA. Example:

      - name: Checkout PR
        if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target'}} 
        uses: actions/checkout@master
        with:
          ref: ${{ github.event.pull_request.head.sha }}
      - name: Checkout
        if: ${{ github.event_name == 'push' }}
        uses: actions/checkout@master

[pfeerick]

Sure thing, I'll give that a try in a different PR, as this was working, but that would be much simpler :grin:

edit: Actually, now that I'm looking at this properly again... that will simply never work, as the GitHub security model is that PR runs from forks do not get write access to the upstream repo (not an issue here) nor do they get access to secrets (the issue here). Hence why the two-step process is required - first step is considered insecure, second step is considered secure.