-
Notifications
You must be signed in to change notification settings - Fork 14
Open
Labels
Description
What happened
- Chris lost a bunch of time to inexplicable behavior in GitHub actions.
- He needed to set a secret in order to raise the rate limits for
github-activity
API calls, because the tests pull github data from repositories outside of the github-activity repository - In the action, a check for
not auth
made it seem like auth didn't exist, even though the variable existed. - I updated github-activity to explicitly check for an empty string condition, and it turns out this was the case! I'm now also checking for
auth is None
instead ofnot auth
. - After a lot of debugging, the problem was that his PR was from his fork of
github-activity
. - Secrets appear as empty strings if the PR is from a fork.
- So the fix here was to re-open the PR from the the base repository, rather than from my fork, and it worked.
- This won't work long-term if others want to open PRs from their forks but we can fix that by ensuring all the tests only pull from that repository, so we don't need API keys
What if we want forked repositories to work?
- You can explicitly get around this with the
pull_request_target
workflow but this is dangerous because others could take advantage of the secret in a PR. - You could still exfiltrate a secret in the code in the PR (e.g. python that prints
os.environ["MY_SECRET"]
) - You can fix this by using
github environments
which let you restrict secrets based on the job. - That way, you can ensure only the secrets that aren't critical (like PYPI publishing) are available to the job
- That's what we did in
github-activity
.
Why is it valuable
- It's common in open source to fork repositories, and this is a silent error which led to so much confusion on my part.
- We added an explicit check for "empty string" in
github-activity
which would have helped here so this will be less brittle and confusing. - Hopefully this post explains some background and helps others avoid it in the future.
Links to learn more
- https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#controlling-changes-from-forks-to-workflows-in-public-repositories
- The broken PR from my fork: Change token to readonly PAT so we don't hit rate limits executablebooks/github-activity#134
- The same PR working from a non-fork: Add a token to avoid rate limits on tests executablebooks/github-activity#135