GitHub Actions update: Helping maintainers combat bad actors | Hacker News Reader

GitHub Actions update: Helping maintainers combat bad actors

github.blog

179 pointstodsacerdoti49 comments

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.

> It wasn’t too long ago that getting CI/CD set up for a small project would either be non-trivial and/or expensive.

You mean over 10 years ago? Travis-CI opened in 2011...

CI/CD is RCE as as service. Not opening CI/CD to the public by default seems like a security necessity.

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.

Hacker news is spam as a service, not letting the public post is a necessity.

Or we could acknowledge there is a space between complete unmanaged access and no access.

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.

> 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?

> 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.

It's really for their benefit rather than maintainers. The actions abuse was apparently a major capacity issue.

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.

"accounts may be blocked"... if they mean "CI may wait in queue" then it was a very poor choice of words

> This obviously has a negative impact on repository owners whose legitimate pull requests and accounts may be blocked as a result of this activity.

They’re saying that the malicious actions block builds for legit users.

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.

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.

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.

Running CI on their repo doesn't work when you use self-hosted runners.

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.

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.

Thanks for your comment I guess?

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.

...maaaayyyyybe.

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.

People already do that regardless of this new feature.

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.

0: https://github.com/jstrieb/ctf-collab

The underlying cost of actions based on Azure compute pricing is about two cents per hour, it's really nothing to feel guilty about.

Might be a good candidate for some self hosted runners on some vms or a pi cluster? Then no need to feel guilty at all

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.

[edit] formatting

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.

This is already possible.

> In the next commit the PR author exfiltrates GitHub secrets by base64ing them and logging in the workflow run (or any other way).

Environment secrets are not exposed to PRs, so this does not work. This really only concerns DoS.

IIRC I can look at the secret names in the workflow definition then pipe them through `base64 | zip` and have fun.

I did this quite some time ago but they may have added better limits in place.

> This is already possible.

How? I did not find it possible to turn off pull requests when I last looked. Is that something they've added recently?

You're right, likely the OP thought PRs were included in settings where you can disable issues or wikis

Discussion: https://github.com/dear-github/dear-github/issues/84

Thanks for correcting me. Looks like I got confused.

Though ironically people have used GitHub Actions to auto-close all incoming pull-requests. XD

Any review should be part of the code review process - not the CI build process.

If the build is broken, a review is likely to be less reliable.

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.

I think blocking the PR author instead of the original project is a good move. Adding more manual labor to maintainers, not so much.

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.

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.

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?

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.

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.

Absolutely. My team was hesitant to even use Github actions until now.

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.

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.

Above all else, before you get your hackles raised, thank GitHub for giving you as much free services as they do.

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.

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.

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.

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.

Your decade of history means nothing to a maintainer, nor it means that you won't, one day, misuse the maintainer's resources.

> 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.

GitHub Actions update: Helping maintainers combat bad actors

Keyword helping. How is this helping maintainers, exactly? Forcing them to perform manual steps or else is not helping. This is pure PR spin.

> 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?

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).

Reply on news.ycombinator.com