Re:GitHub Actions update: Helping maintainers combat bad actors
by shubik22 on Thursday April 22, 2021 @10:04PM
(#26910295)
Kinda surprised about the negativity here. It wasn’t too long ago that getting CI/CD set up for a small project would either be non-trivial and/or expensive. The value I get (for free!) from Github Actions is pretty substantial. Sounds like their previous policy of blaming upstream maintainers didn’t make sense, and now they’re trying to strike a reasonable balance between preventing abuse and burdening maintainers here. Maybe you think they’ve swayed too far to one side here, but I’m inclined to give Github the benefit of the doubt given the overwhelming value that I think Github Actions provides projects for free.
Re:GitHub Actions update: Helping maintainers combat bad actors
by Longwelwind on Friday April 23, 2021 @10:46AM
(#26915261)
I'm building a project where my service would plug with the (unstrusted) JS code of users.
I first thought it would be easy to sandbox and have something decent running, but after making some research on sandboxing, I realize how hard it is, and the many ways bad actors can exploit a service running untrusted code.
Kudos to GitHub and GitLab for taking the challenge of providing a RCE service with a free plan.
Re:GitHub Actions update: Helping maintainers combat bad actors
by rdsubhas on Friday April 23, 2021 @05:51AM
(#26912899)
Poor analogy. CI is about automating what a developer does remotely, so yes CI IS automated code execution in a remote environment, as a service. But Hacker news is "spam as a service" – huh?? They don't even compare.
Secondly, parent comment said public by default is bad. Parent didn't say "all public was bad". So there is no necessity to make it into a disagreement.
Re:GitHub Actions update: Helping maintainers combat bad actors
by remram on Thursday April 22, 2021 @07:39PM
(#26909447)
> One of these is pull requests from forks being used by bad actors to run mining code on upstream repositories. This obviously has a negative impact on repository owners whose legitimate pull requests and accounts may be blocked as a result of this activity.
It's really not obvious to me why you'd ever punish one account/repository for the actions of another account in another repository...
> pull requests from first-time contributors will require manual approval
So GitHub is... forcing maintainers to fight the abuse of GitHub's own resources by third parties... or else they will ban the maintainers? What is this?
Re:GitHub Actions update: Helping maintainers combat bad actors
by ncallaway on Thursday April 22, 2021 @09:26PM
(#26910091)
> It's really not obvious to me why you'd ever punish one account/repository for the actions of another account in another repository...
Well, from the blog post, that's one of the changes they outlined:
> Specifically, if we determine an Actions run to be abusive or against our terms, our enforcement will be directed at the account hosting the fork and not the account associated with the upstream repository
My guess is the reputation was a naive initial implementation, designed to prevent against someone opening a Github account, creating a mining action and running it. They probably just didn't consider these scenarios carefully when first applying reputation to pull requests, and this blog posts is them fixing it.
Re:GitHub Actions update: Helping maintainers combat bad actors
by simtel20 on Thursday April 22, 2021 @09:09PM
(#26909980)
I think you are reading "blocked" as in "account suspended", while I think the article means that other users' jobs are blocked - meaning waiting - as the mining code is running instead of completing in a short time as you'd normally expect.
Re:GitHub Actions update: Helping maintainers combat bad actors
by remram on Thursday April 22, 2021 @10:16PM
(#26910354)
Yes simtel20 just said that. Hopefully it is what they meant.
Either way I am now responsible for protecting GitHub's CI resources and I don't particularly like it; I'll do it if I must (after all GitHub is generously providing them for free) but I think the post's title is a bit dishonest.
Re:GitHub Actions update: Helping maintainers combat bad actors
by akerl_ on Thursday April 22, 2021 @10:47PM
(#26910508)
It’s not really in dispute that it’s what they mean. And thus, you aren’t responsible for protecting GitHub’s infra; they’re providing new options for ensuring your own Actions setup isn’t subverted for somebody to mine crypto without your consent.
Re:GitHub Actions update: Helping maintainers combat bad actors
by remram on Thursday April 22, 2021 @11:57PM
(#26910837)
My objection is simple: I have to protect my CI queue simply because GitHub chose to count PRs from third party against my project's queue. If they simply counted them against the PR author's quota, this abuse wouldn't be happening in the first place. People would run them in discreet repos as they would gain nothing from forking or opening PRs.
Now instead of counting this CI usage in a way more favorable to project maintainers, they give us a way to manually approve runs before they use our CI. That's good, but still a solution to an artificial problem. I think I would have welcomed more openness about why this way of accounting is necessary, instead of this extra work put on me and labeled as a "help" that I never thought I needed.
Re:GitHub Actions update: Helping maintainers combat bad actors
by remram on Friday April 23, 2021 @10:39AM
(#26915205)
Obviously I don't consider self-hosted runners to be "GitHub's resources". However those are not widely used by open-source projects and are not mentioned in the blog post, so I don't think that's what the concern is about. I don't think ignoring this minor use-case makes this a "knee-jerk reaction", but name-calling is always appreciated...
I don't see how secrets help with mining cryptocurrency either.
Re:GitHub Actions update: Helping maintainers combat bad actors
by qbasic_forever on Thursday April 22, 2021 @09:52PM
(#26910228)
Ooph, yeah I can see this turning into a big headache for large community repos. People are going to submit a first pull and then start opening issues like "Why hasn't pull request #123 been approved yet?, "Please approve my pull #456", etc. within minutes and hours.
Re:GitHub Actions update: Helping maintainers combat bad actors
by jstrieb on Thursday April 22, 2021 @10:52PM
(#26910531)
One of my own projects sits in a related grey area: it automates running a reverse shell inside an Action for collaborating on programming and competitive hacking challenges/CTF problems.[0] It lets anyone log into a shared session from the terminal or the browser.
My project doesn't use anywhere near the resources of cryptomining, but I still sometimes feel guilty when I fire it up. I wonder how different it is from regular GitHub Actions usage patterns, and whether it is noticeable to those maintaining the infrastructure. My hope is that the load it incurs is comparatively insignificant, and that if noticed, it will be viewed as a good-faith attempt to use resources in a creative way.
Re:GitHub Actions update: Helping maintainers combat bad actors
by sramam on Friday April 23, 2021 @03:21AM
(#26911993)
I am sympathetic to their need for manual intervention, but maintainers should be the last line of defense, not the first and only line they seem to have here. Specifically:
> If the pull request is updated to a new commit, a new approval will be required.
What?!
It'd be much more palatable if the manual approvals were more selective.
For example:
- only invoked if a job takes more than 1.5x or 2x the max recorded run-time for the job.
- some notion of PR-author's karma being used to throttle manual approvals from maintainers.
It also would be fair to allow maintainers that do not want this load to mark their repos as open-source but closed to contributions.
Re:GitHub Actions update: Helping maintainers combat bad actors
by hashhar on Friday April 23, 2021 @06:43AM
(#26913259)
There are valid reasons for requiring re-approval.
A very good example is that a normal looking first commit gets approved.
In the next commit the PR author exfiltrates GitHub secrets by base64ing them and logging in the workflow run (or any other way).
Boom - now your CI infra AWS S3 testing bucket, Google BigQuery keys etc. are leaked.
Runtime is also easy to work-around - repeatedly push commits with a execution time limit on the CI workflow to avoid triggering outlier detection.
A trusted PR author today doesn't guarantee trust tomorrow.
Also your trust on some other repo doesn't (and shouldn't) transfer across repos.
> It also would be fair to allow maintainers that do not
> want this load to mark their repos as open-source but
> closed to contributions.
Re:GitHub Actions update: Helping maintainers combat bad actors
by codeflo on Friday April 23, 2021 @08:03AM
(#26913828)
It's essentially an admission that the "mitigations" they claim to have implemented (second paragraph) aren't that effective, and they want to outsource the review effort to the community. Which is fine in my opinion, but I agree that the current tools might not be fine granular enough. There should also be clearer messaging about what happens when there's an honest review mistake and a crypto miner slips through.
Re:GitHub Actions update: Helping maintainers combat bad actors
by miohtama on Friday April 23, 2021 @07:42AM
(#26913678)
After Github has enough data, it can automatically identify malicious contributors. Facebook, Twitter, already do this for spam. But you are still going to need manual reports to get data.
Re:GitHub Actions update: Helping maintainers combat bad actors
by paxys on Friday April 23, 2021 @12:31AM
(#26911014)
Is it really even marginally profitable (compared to the effort put in) to run crypto mining scripts on the tiny amount of CPU allocated to CI runs in shared pools?
Re:GitHub Actions update: Helping maintainers combat bad actors
by tmp538394722 on Thursday April 22, 2021 @11:34PM
(#26910718)
I already require manual intervention via a bot interaction to trigger CI, so this all SGTM.
It seems like kind of a stretch to be outraged by this. I suppose there might be certain styles of collaborative repositories that have a huge volume of one time contributors, but I’m having a hard time imagining this being much of a burden.
Re:GitHub Actions update: Helping maintainers combat bad actors
by gtirloni on Thursday April 22, 2021 @10:08PM
(#26910314)
Surprised it took this long to implement this feature, it's great. It's how we did things back in the Jenkins days and it's absolutely necessary. Kudos.
Re:GitHub Actions update: Helping maintainers combat bad actors
by orf on Friday April 23, 2021 @01:46PM
(#26917237)
This approval process kills the utility of Actions on mature projects where a very limited number of people have write access but has a fairly high volume of contributions, such as Rails and Django.
Quite disappointed. I assume GitHub has quite a few reputational indicators they could use, requiring approval for all first time contributors is a very blunt hammer.
Re:GitHub Actions update: Helping maintainers combat bad actors
by chr1 on Friday April 23, 2021 @07:23AM
(#26913528)
They know how much time and resources a typical ci run for the project is going to take, so why not limit pull requests from new users to that time, and ask the maintainer to manually approve only if that time is exceeded? This way most of normal users will not notice any change, and miners won't have enough time to cause harm.
Re:GitHub Actions update: Helping maintainers combat bad actors
by msoad on Thursday April 22, 2021 @08:52PM
(#26909894)
This is a lazy fix. Why don't you asses reputation of users? I have a decade of history on my GitHub account with lots of projects and contributions. Why should my PR not run CI if I make a change to a new repository?
I bet they invited a huge team of machine learning and "AI" experts to do those user assessments that will launch in fall of 2029. Those models will always fail in one of or two edge cases and will cause embarrassments.
Re:GitHub Actions update: Helping maintainers combat bad actors
by Arnavion on Thursday April 22, 2021 @09:34PM
(#26910120)
If you're making a PR to my repository, I would like to be in control of whether my repository's resources are used for your PR or not. It doesn't matter whether you've had your account for 10 minutes or 10 years.
(And they certainly are "my repository's resources", since the build machines pool is shared between workflow runs for branches and PRs.)
They're codifying what a lot of repositories do with bots today anyway, where a maintainer triggers the CI to run on a PR by posting a comment.
Also I'm confused how you can simultaneously berate them for not building a user reputation system, and then berate them for hypothetically building a user reputation system.
Re:GitHub Actions update: Helping maintainers combat bad actors
by arcatek on Friday April 23, 2021 @05:01AM
(#26912600)
I've contributed to a lot of repositories, and I've yet to see a single one where tests aren't automatically run. I can only think of a single one where some benchmarks are run on-demand.
Re:GitHub Actions update: Helping maintainers combat bad actors
by hashhar on Friday April 23, 2021 @06:45AM
(#26913271)
Most of the repositories under orgs. like ASF or CNCF are default-deny. They require action from maintainers before a build gets triggered. Mozilla is the same way.
Re:GitHub Actions update: Helping maintainers combat bad actors
by ncallaway on Thursday April 22, 2021 @10:41PM
(#26910475)
> Forcing them to perform manual steps or else is not helping
I don't see the "forcing" or the "or else" in the github blog post.
Can you explain how github is forcing them to perform a manual step? Or, can you explain what the consequence of them not performing the manual step of approving the PR is?
Re:GitHub Actions update: Helping maintainers combat bad actors
by Gaelan on Friday April 23, 2021 @12:26AM
(#26910990)
Previously, PRs would automatically cause actions to be run, for all users. So users could notice and fix CI failures themselves, and maintainers could see that CI was passing they first looked at the PR. The maintainer needs to touch the PR a minimum of once: look at the code, see CI passes, merge. Now, the maintainer has to look at the code and approve, at which point they have to wait for CI to pass, then come back later and merge; or, if CI fails, come back and re-approve each time the contributor tries to fix it, so a minimum of two times, and quite possibly more if there are CI failures (which, I expect, is common for first-time contributions).
You mean over 10 years ago? Travis-CI opened in 2011...
I first thought it would be easy to sandbox and have something decent running, but after making some research on sandboxing, I realize how hard it is, and the many ways bad actors can exploit a service running untrusted code.
Kudos to GitHub and GitLab for taking the challenge of providing a RCE service with a free plan.
Or we could acknowledge there is a space between complete unmanaged access and no access.
Secondly, parent comment said public by default is bad. Parent didn't say "all public was bad". So there is no necessity to make it into a disagreement.
It's really not obvious to me why you'd ever punish one account/repository for the actions of another account in another repository...
> pull requests from first-time contributors will require manual approval
So GitHub is... forcing maintainers to fight the abuse of GitHub's own resources by third parties... or else they will ban the maintainers? What is this?
Well, from the blog post, that's one of the changes they outlined:
> Specifically, if we determine an Actions run to be abusive or against our terms, our enforcement will be directed at the account hosting the fork and not the account associated with the upstream repository
My guess is the reputation was a naive initial implementation, designed to prevent against someone opening a Github account, creating a mining action and running it. They probably just didn't consider these scenarios carefully when first applying reputation to pull requests, and this blog posts is them fixing it.
They’re saying that the malicious actions block builds for legit users.
Either way I am now responsible for protecting GitHub's CI resources and I don't particularly like it; I'll do it if I must (after all GitHub is generously providing them for free) but I think the post's title is a bit dishonest.
Now instead of counting this CI usage in a way more favorable to project maintainers, they give us a way to manually approve runs before they use our CI. That's good, but still a solution to an artificial problem. I think I would have welcomed more openness about why this way of accounting is necessary, instead of this extra work put on me and labeled as a "help" that I never thought I needed.
Same for when you have secrets defined in the workflow that are necessary for a build.
There are more cases to cover than the ones visible during a knee-jerk reaction.
I don't see how secrets help with mining cryptocurrency either.
Thanks for your comment I guess?
But on the other hand, it makes it less likely that crypto miners will bother opening up junk PRs against your repo. So you save some time there.
My project doesn't use anywhere near the resources of cryptomining, but I still sometimes feel guilty when I fire it up. I wonder how different it is from regular GitHub Actions usage patterns, and whether it is noticeable to those maintaining the infrastructure. My hope is that the load it incurs is comparatively insignificant, and that if noticed, it will be viewed as a good-faith attempt to use resources in a creative way.
0: https://github.com/jstrieb/ctf-collab
> If the pull request is updated to a new commit, a new approval will be required.
What?!
It'd be much more palatable if the manual approvals were more selective. For example:
- only invoked if a job takes more than 1.5x or 2x the max recorded run-time for the job.
- some notion of PR-author's karma being used to throttle manual approvals from maintainers.
It also would be fair to allow maintainers that do not want this load to mark their repos as open-source but closed to contributions.
[edit] formatting
A very good example is that a normal looking first commit gets approved. In the next commit the PR author exfiltrates GitHub secrets by base64ing them and logging in the workflow run (or any other way).
Boom - now your CI infra AWS S3 testing bucket, Google BigQuery keys etc. are leaked.
Runtime is also easy to work-around - repeatedly push commits with a execution time limit on the CI workflow to avoid triggering outlier detection.
A trusted PR author today doesn't guarantee trust tomorrow. Also your trust on some other repo doesn't (and shouldn't) transfer across repos.
> It also would be fair to allow maintainers that do not > want this load to mark their repos as open-source but > closed to contributions.
This is already possible.
Environment secrets are not exposed to PRs, so this does not work. This really only concerns DoS.
I did this quite some time ago but they may have added better limits in place.
How? I did not find it possible to turn off pull requests when I last looked. Is that something they've added recently?
Discussion: https://github.com/dear-github/dear-github/issues/84
Though ironically people have used GitHub Actions to auto-close all incoming pull-requests. XD
If the build is broken, a review is likely to be less reliable.
I don’t have an alternative solution, though. I wish it would at least pass through PRs opened from GitHub citizens with some form of reputation.
It seems like kind of a stretch to be outraged by this. I suppose there might be certain styles of collaborative repositories that have a huge volume of one time contributors, but I’m having a hard time imagining this being much of a burden.
Quite disappointed. I assume GitHub has quite a few reputational indicators they could use, requiring approval for all first time contributors is a very blunt hammer.
I bet they invited a huge team of machine learning and "AI" experts to do those user assessments that will launch in fall of 2029. Those models will always fail in one of or two edge cases and will cause embarrassments.
(And they certainly are "my repository's resources", since the build machines pool is shared between workflow runs for branches and PRs.)
They're codifying what a lot of repositories do with bots today anyway, where a maintainer triggers the CI to run on a PR by posting a comment.
Also I'm confused how you can simultaneously berate them for not building a user reputation system, and then berate them for hypothetically building a user reputation system.
> Why should my PR not run CI if I make a change to a new repository?
Because the maintainer doesn't know you and certainly doesn't have time to read through your 10 years of contribution.
Keyword helping. How is this helping maintainers, exactly? Forcing them to perform manual steps or else is not helping. This is pure PR spin.
I don't see the "forcing" or the "or else" in the github blog post.
Can you explain how github is forcing them to perform a manual step? Or, can you explain what the consequence of them not performing the manual step of approving the PR is?