GitHub Actions update: Helping maintainers combat bad actors

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.

[edit] formatting

[ Parent | on HN ]

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.

This is already possible.

[ Parent | on HN ]

Re:GitHub Actions update: Helping maintainers combat bad actors by mhils on Friday April 23, 2021 @07:19AM (#26913503)

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

[ Parent | on HN ]

Re:GitHub Actions update: Helping maintainers combat bad actors by hashhar on Monday April 26, 2021 @01:49AM (#26939602)

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.

[ Parent | on HN ]

Re:GitHub Actions update: Helping maintainers combat bad actors by gray_-_wolf on Friday April 23, 2021 @07:31AM (#26913581)

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

[ Parent | on HN ]

Re:GitHub Actions update: Helping maintainers combat bad actors by __s on Friday April 23, 2021 @08:03AM (#26913823)

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

[ Parent | on HN ]

Re:GitHub Actions update: Helping maintainers combat bad actors by hashhar on Monday April 26, 2021 @01:52AM (#26939618)

Thanks for correcting me. Looks like I got confused.

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

[ Parent | on HN ]

Re:GitHub Actions update: Helping maintainers combat bad actors by sramam on Friday April 23, 2021 @07:52AM (#26913749)

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.

[ Parent | on HN ]

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.

[ Parent | on HN ]