openzim
Make CI work even for external contributors
Due to Github good security guardrails, many secrets are not passed properly when an external contributor opens a PR from its own clone. This is made for security concern. I'm not sure how we can adapt to this without breaking security, but we need to find a solution for external contributors PRs to still pass.
See https://github.com/openzim/youtube/actions/runs/12014257728
The approach should be that each PR by an external contributor is authorised manually by people from the "Lieutenants team" https://github.com/orgs/openzim/teams/lieutenants to run the CI.
Current situation is not really a bug, but a feature: we don't want to run blindly external PR against our CI.
No, current approach (the one in place or the one you suggest are mostly identical) does not work properly, secrets are not passed to the runner when a workflow is triggered from a forked repository ; see https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions#using-secrets-in-a-workflow
This is why codecov uploads are often failing in PRs from external contributors, and why youtube CI is systematically failing because we need access to one Youtube API key for running integration tests
@benoit74 In which repo it fails to run properly?
Actually i believe to know there is at least one which works fine... but I don't remember where is this implemented exactly.
This repo is not working properly (or more exactly the fork from https://github.com/openzim/youtube/pull/371)
secrets are not passed to the runner when a workflow is triggered from a forked repository
What are you validating for if there's no secrets passing? The GH-correct way to do this when there are secrets involved are to put the required secrets in an Environment secrets ; make the CI use that environment when run from an external contributor and require approval in this case.
That's what we do for apple. Works fine.
What are you validating for if there's no secrets passing?
Secrets are needed by the CI job, configured in the definition, but Github does not pass the secrets with the PR is from an external contributor.
I don't get why Environment secrets would be passed to the job, they are still secrets... Let's give it a try.
I think that what make it work on kiwix-apple is not the environment but the fact that pull_request_target event is used instead of pull_request. I'm not an expert on this at all, but it looks like Github strongly recommend to be very cautious with pull_request_target: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull_request_target
Of course it is. That's why every run that involve secrets have to be manually authorized by someone (a lieutenant in apple's case) because it's strictly not possible to provide secrets to a workflow in a way that it cant be revealed by a malicious or accidental way.
The best solution is always to not use secrets in workflow.
Of course.
But then I don't get why we did not deployed this on all repos, since they (mostly all) use the CODECOV_TOKEN secret, and not having this secret is probably the cause of all instabilities we observed on these codecov actions (the action work "randomly" when CODECOV_TOKEN is not set). Are we ok that we need to generalize kiwix-apple approach based on pull_request_target on all our repos (and in particular in python bootstrap: https://github.com/openzim/_python-bootstrap/blob/main/.github/workflows/Tests.yaml)?
It was implemented last year only. Apple is different to scrapers in that secrets are very sensitive (compared to say CODECOV_TOKEN) and we had frequent external contributors.
I don't know if/how it's implemented in the repos I dont interact much with (mwoffliner, libzim/kiwix, kiwix-desktop and kiwix-tools).
On scrapers, if the only concern is the codecov_token, it might be worth investigating it directly. It is supposed to work reliably (and it did in the past) without the secret. Maybe we need to do something else to fix it without getting into this cumbersome validation just to get a coverage feedback.
codecov_token is not the only concern on scrapers, but is is indeed the main one in most cases. For youtube, we also have an API_KEY secret for Youtube) which is way more sensitive.
AFAIK, codecov action is not working reliably without the codecov token due to rate limiting, see e.g. https://github.com/openzim/ted/actions/runs/11307130547/job/31499018798 at codecov ; this was with action v3, maybe this concern is now gone with v5, it looks like we now even have an new option for tokenless upload: https://github.com/codecov/codecov-action). Worth investigating indeed
Not worth it until we've investigated codecov behavior with v5 action to clarify how "mandatory" the CODECOV_TOKEN is for this action.
And so far I feel like we are quite aligned with rgaudin, there is discussion but only moving forward for now. Do you mean you would like to discuss this in a meeting with us?
For the record, it looks like using an environment is not the solution, see https://github.com/openzim/youtube/actions/runs/13919085187/job/38947903053?pr=388 which ran on https://github.com/openzim/youtube/pull/388/commits/c67bf0f67176b86491801072c72b5de496cc2057
The best workaround I've found so far is that I push myself to a branch on origin repo and open a "ghost" PR ; since commit is identical, result of workflows are shown in both PR, and since I'm a maintainer secrets are passed to the workflow 😩