ci: Migration from AppVeyor to GitHub Actions

hebasto commented at 1:53 pm on December 27, 2019: member

From the IRC meeting on 2019-12-12 log:

19:08:17 <wumpus> #topic appveyor CI future (sipsorcery) … 19:25:14 <wumpus> in any case, let’stest it and talk about it next meeting or so 19:25:26 <MarcoFalke> Ok, so unless there are objections: Merge #17736 as short term fix, and then (next week maybe) switch to GitHub Actions CI? 19:25:28 <gribble> #17736 | Update msvc build for Visual Studio 2019 v16.4 by sipsorcery · Pull Request #17736 · bitcoin/bitcoin · GitHub 19:25:41 <sipsorcery> +1 19:25:49 <fanquake> +1 19:25:49 <wumpus> ack 19:25:52 <emilengler> +1

Is it time to set up a CI workflow in this repo?

Pros: see meeting log. Cons: to be built successfully a PR should be rebased on top of #17793.

MarcoFalke commented at 3:06 pm on December 27, 2019: member

Some questions:

It was claimed that this speeds up the build. Is this true?
Who is going to be able to reset builds? (For appveyor it is only me, I believe. So anything other than that is an improvement)
There were concerns that we shouldn’t put all eggs in one basket (GitHub), but it seems they are free and for appveyor we pay 50$ per month for two parallel jobs

fanquake added the label Tests on Dec 27, 2019

hebasto commented at 4:07 pm on December 27, 2019: member

@MarcoFalke

It was claimed that this speeds up the build. Is this true?

Yes, it is. But not substantially: ~5..10% (this estimation is based on builds in my account).

If cache is invalidated, additional time up to 30 minutes is required (same as AppVeyor). But this does not cause timeout.

Who is going to be able to reset builds?

Managing a workflow run - About workflow permissions:

You must have at least write-level access to cancel or re-run workflow runs.

sipsorcery commented at 10:48 pm on December 31, 2019: member

It was claimed that this speeds up the build. Is this true?

I’ve found GitHub builds are 10 mins faster (Appveyor between 50-60 mins, GH between 40-50 mins). GH also lets builds run over 1 hr and permits parallel jobs (I know the Bitcoin Core CI job has a special arrangement with AppVeyor but jobs on my fork regularly timeout).

There were concerns that we shouldn’t put all eggs in one basket (GitHub), but it seems they are free and for appveyor we pay 50$ per month for two parallel jobs

A possible plan is:

Enable GitHub Actions build for master only and leave AppVeyor in place (I believe the “tick” for a PR will depend on whether GH Actions is enabled for PR’s or not).
If no problems enable GH Actions for PR’s. That will impact contributors as they now have three CI systems that need to pass to get a green tick,
If no problems evaluate whether it’s worth cancelling AppVeyor and saving the $50/month.

MarcoFalke commented at 4:41 pm on January 2, 2020: member

It looks like GitHub Actions (when granted) gives any permission to modify the repository. This is overkill and harmful. A CI should be read-only, not be allowed to push arbitrary code changes (or even just modify labels)

laanwj commented at 4:53 pm on January 2, 2020: member

Yes the worry is that it would allow PRs that modify the CI instructions to make (indirectly) arbitrary changes to the repository, e.g. through ${{ secrets.GITHUB_TOKEN }}. I’d like to be really sure that this is not possible before activating third-party actions.

sipsorcery commented at 5:02 pm on January 2, 2020: member

Do you mean the GHA can make commits if the access token is set? That’s the same as AppVeyor (and probably most CI’s).

The token has to be set by a repo maintainer. Without it an action won’t be able to make any commits.

MarcoFalke commented at 5:10 pm on January 2, 2020: member

The token has to be set by a repo maintainer

Are you sure? https://github.com/bitcoin/bitcoin/settings/secrets is empty (for me in my fork) and I could add a new label to a pull request with an action.

sipsorcery commented at 5:28 pm on January 2, 2020: member

https://github.com/settings/tokens is where I have got tokens that allow one of my AppVeyor jobs to make automatic commits.

But as to whether there are other ways that a repo can be modified, no I’m not sure. I also can’t find why the GHA on this repo is being triggered by issues comments (like this one). On my fork only pushes trigger the job.

MarcoFalke commented at 5:38 pm on January 2, 2020: member

https://github.com/settings/tokens does not contain a token named GITHUB_TOKEN either.

Screenshot_2020-01-02 Build software better, together

sipsorcery commented at 5:45 pm on January 2, 2020: member

But isn’t that the point? Unless a token is explicitly created and added to the GHA workflow file it won’t be able to make commits.

Here’s what it looks like for AppVeyor. I manually created the token on my GitHub settings\tokens page and then encrypt it for my AppVeyor account settings. With the token the AppVeyor job can make commits on a specific branch in one of my repos. Without it my understanding is commits can’t be made.

0version: 1.0.{build}
1environment:
2  APPVEYOR_SAVE_CACHE_ON_ERROR: true
3  access_token:
4    # Token for github pages publishing.
5    secure: 21m76jAVvcu7oACAHFnfCBltcwon+r5ZI3avfRmrNFAqJMn6RfLXwpBhPcJ617tD
6image: Visual Studio 2019

My expectation is that GHA is the same as AppVeyor. But I’m not sure. I’ll post a question on the GitHub support forum.

https://github.community/t5/GitHub-Actions/Can-an-Action-make-commits-without-explicitly-setting-an-access/m-p/42383#M4973

sipsorcery commented at 6:00 pm on January 2, 2020: member

@MarcoFalke looks like setting the Actions Permissions to Enable local Actions only for this repository causes the Action to be triggered for isuse comments (and probably every other report interactions). This is not what we want and it might be better to disable the Action. ~~while I track down why this behaviour occurs.~~

Update: Most likely cause for the behaviour is that GitHub triggers the Action on every event and then lets the workflow file filter them. By setting the Enable local Actions only for this repository permission it causes an error immediately since the ci.yml file on this repo is using 3rd party actions. The Action is going to permanently fail until the permission is changed or 3rd party actions are removed. Recommendation is to disable pending further investigation into permissions.

Setting the permission to Enable local and third party Actions for this repository results in my fork only triggering on Pull requests. I think that permission level simply means 3rd party actions like the cache and checkout ones can be used in a workflow file. But until we know for sure it rules out potential for commits might be best to disable Actions again.

MarcoFalke commented at 6:43 pm on January 2, 2020: member

I am using the placeholder, so I am not explicitly setting a token:

https://github.com/MarcoFalke/bitcoin-core/blob/21dd161ef18a1c0b88269ee5ddbcd7c0dbceffc5/.github/workflows/label.yml#L19

Then it was somehow able to add a label: https://github.com/MarcoFalke/bitcoin-core/pull/10#event-2918185136

sipsorcery commented at 7:34 pm on January 2, 2020: member

Turns out an automatic token is available for a GitHub Action. That’s not good for the Bitcoin Core repo as that token does have write permissions.

MarcoFalke’s comment below is thus very valid.

It looks like GitHub Actions (when granted) gives any permission to modify the repository. This is overkill and harmful. A CI should be read-only, not be allowed to push arbitrary code changes (or even just modify labels)

Maybe there’s a way to disable write permissions for the Actions App but I’m yet to find it.

MarcoFalke commented at 8:10 pm on January 2, 2020: member

Reported upstream: https://github.community/t5/GitHub-Actions/Read-only-token-for-CI/m-p/42393#M4978

MarcoFalke added the label Upstream on Jan 3, 2020

sipsorcery commented at 10:53 am on January 4, 2020: member

Assuming the answer on MacroFalke’s question is correct and there is no way to disable the default token then I think using a GitHub Action build on Bitcoin Core is a NACK.

The risk could be managed by replacing 3rd party actions with custom actions but that’s way too big a burden for the benefits over the existing AppVeyor build.

NACK until a GitHub Action readonly token option is available.

Sjors commented at 6:00 pm on January 28, 2020: member

Fortunately we have the ear of the GitHub CEO: #15847

MarcoFalke commented at 6:42 pm on January 30, 2020: member

I don’t think they ever made changes based on our feedback.

It would probably be good to remove the github ci script again.

fanquake referenced this in commit d176aeafde on Jan 31, 2020

fanquake commented at 10:22 pm on February 4, 2020: member

Going to close for now, given we’ve removed the GitHub Actions workflow (#18031).

fanquake closed this on Feb 4, 2020

real-or-random commented at 5:20 pm on November 11, 2020: member

Yes the worry is that it would allow PRs that modify the CI instructions to make (indirectly) arbitrary changes to the repository, e.g. through ${{ secrets.GITHUB_TOKEN }}. I’d like to be really sure that this is not possible before activating third-party actions.

That’s not possible as I understand it.

This blog post clarifies that pull requests from forks only get a read-only token: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks (see in particular the first paragraph in the second section… the entire section section is about dealing with workflows that need write access but currently do not have it ):

In order to protect public repositories for malicious users we run all pull request workflows raised from repository forks with a read-only token and no access to secrets.

But the docs are bad here. That behavior is not at all clear from https://docs.github.com/en/free-pro-team@latest/actions/reference/encrypted-secrets#using-encrypted-secrets-in-a-workflow and https://docs.github.com/en/free-pro-team@latest/actions/reference/authentication-in-a-workflow#permissions-for-the-github_token … Maybe I’ll create a PR to their docs. If we’re in doubt, we could just try on a new repo.

Were there further concerns? If not, GitHub Actions is again an interesting option. (Not saying we should do this, I’m at the moment just looking into options for secp256k1 (https://github.com/bitcoin-core/secp256k1/issues/707).

MarcoFalke commented at 5:54 pm on November 11, 2020: member

the docs are bad

We don’t have the resources to read through outdated and incorrect docs to decide whether a token has read or write permissions. Even if the docs were clear that some token has only read permissions attached, it is probably vulnerable in the implementation. See also https://bugs.chromium.org/p/project-zero/issues/detail?id=2070

Hosting the code, managing access control and running untrusted CI scripts on the same server/domain is just going to leave us dealing with commits such as this one (https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57) in master

Even if there was no vulnerability, with software today moving to shorter release cycles and speeding up the release process by having the ci directly deploy the code is demanding GitHub to offer write tokens at some point (if they haven’t already).

Moreover, GitHub doesn’t even allow us to assign our contributors the right permissions to manage issues, labels and projects. I highly doubt they have figured out the permissions for ci tokens in a way that makes sense for us.

PastaPastaPasta referenced this in commit 9222044ca7 on Jun 27, 2021

PastaPastaPasta referenced this in commit 8263b7c62b on Jun 28, 2021

PastaPastaPasta referenced this in commit a1d285c46a on Jun 29, 2021

PastaPastaPasta referenced this in commit d55e5845fc on Jul 1, 2021

PastaPastaPasta referenced this in commit ed9342b8bf on Jul 1, 2021

DrahtBot locked this on Feb 15, 2022

ci: Migration from AppVeyor to GitHub Actions #17803