Skip to content

Conversation

@flavorjones
Copy link

@flavorjones flavorjones commented Dec 30, 2025

This PR is trying to drive the conversation from https://bugs.ruby-lang.org/issues/21804.

I don't think that it's a good idea to auto-merge on a project so critical in the Ruby supply chain. For a foundational action that runs across thousands of CI pipelines, the blast radius of a bad merge is huge.

Auto-merge might be reasonable, but only if it’s tightly scoped to low-risk, mechanically generated changes with strong guardrails (which this PR has tried to do); but if ruby-builder-bot gets compromised then it's game over for the Ruby supply chain.

Pros:

  • Faster propagation of routine updates (e.g., version lists, metadata bumps) without maintainer latency.
  • Less maintainer toil on high-frequency bot PRs.
  • More consistent update cadence and fewer stale PRs.

Cons:

  • Single-point-of-failure risk: a compromised bot or supply-chain attack can push a bad change quickly to many downstream users.
  • Reduced human review on changes that may have subtle security or correctness impacts.
  • Harder to detect abuse if tests can be manipulated or if the update surface grows over time.

This PR is trying to drive the conversation from
https://bugs.ruby-lang.org/issues/21804. I don't know that it's a good
idea to auto-merge on a project so critical in the Ruby supply
chain. For a foundational action that runs across thousands of CI
pipelines, the blast radius of a bad merge is huge.

Auto-merge might be reasonable, but only if it’s tightly scoped to
low-risk, mechanically generated changes with strong guardrails.

Pros:

- Faster propagation of routine updates (e.g., version lists, metadata bumps) without maintainer latency.
- Less maintainer toil on high-frequency bot PRs.
- More consistent update cadence and fewer stale PRs.

Cons:

- Single-point-of-failure risk: a compromised bot or supply-chain attack can push a bad change quickly to many downstream users.
- Reduced human review on changes that may have subtle security or correctness impacts.
- Harder to detect abuse if tests can be manipulated or if the update surface grows over time.
@p-linnane
Copy link

Thanks for taking the time to open this PR to move the discussion forward, even though you’re understandably skeptical of the approach. I share your concerns.

For context, I’m a lead maintainer for Homebrew, where we make extensive use of automerging across our repositories, but only with significant guardrails. Our merge process is more complex than setup-ruby, but automerge is never fully hands-off. It only occurs after explicit human review and approval.

Given the critical role setup-ruby plays in the Ruby supply chain, I think a better path forward is increasing the number of people involved in release management, rather than further reducing human oversight. Adding release managers, as @eregon suggested on the bug tracker, seems like a safer and more sustainable solution.

run: |
gh pr merge "${{ github.event.workflow_run.pull_requests[0].number }}" \
--repo "${{ github.repository }}" \
--squash \
Copy link
Member

@eregon eregon Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
--squash \
--rebase \

Because I always use Rebase and merge in this repo

@eregon
Copy link
Member

eregon commented Dec 30, 2025

Adding release managers, as @eregon suggested on the bug tracker, seems like a safer and more sustainable solution.

To be clear, it's already the case since @hsbt released 4.0.0. It's up to him whether he is OK with that extra work mentioned in https://bugs.ruby-lang.org/issues/21804#note-2.

@eregon
Copy link
Member

eregon commented Dec 30, 2025

This looks good to me.
I would add automatically releasing after the PR is merged (would be nice if we can reuse https://github.com/ruby/setup-ruby/blob/master/.github/workflows/release.yml by "calling it"), otherwise it would still need a manual step.

I think the main downside I see is if master is somehow "broken" yet CI passes and some automated PR is made, merged & released then it can break @v1. But that should be very rare.
There can also be the problem that multiple automated PRs are made and they pass CI but then they are effectively not tested together.

Cons:

  • Single-point-of-failure risk: a compromised bot or supply-chain attack can push a bad change quickly to many downstream users.

I'll check if ruby-builder-bot has 2FA and if not add it.
Assuming it's not possible to login as ruby-builder-bot illegitimately, what are other risks/example attacks?

  • Reduced human review on changes that may have subtle security or correctness impacts.

I think it would be good to add a guard that only some files are modified, specifically:
Looking at https://github.com/ruby/setup-ruby/pull/844/files

  • README.md
  • ruby-builder-versions.json
  • dist/index.js (which is checked in CI to be matching)

Those are very low risk, i.e. any attacker editing those AFAIK still couldn't do anything bad with it except making workflow fails (e.g. by removing a version). They are all metadata files (.json/.md), or checked generated files, no code.
We still need to validate the contents of ruby-builder-versions.json so it's only basenames and e.g. no /.

And for Windows:
Looking at https://github.com/ruby/setup-ruby/pull/847/files

  • windows-toolchain-versions.json
  • windows-versions.json

Those need some validation (or a refactor), because they contain URLs.
We could check these URLs start with https://github.com/ruby/setup-msys2-gcc/releases/ / https://github.com/oneclick/rubyinstaller2/releases/download/ in e.g. windows.js.
But that's prone to "URL injection" so we should also only have basenames there, and validate them.

@flavorjones Maybe you could amend the PR to check only those 5 files are modified?

  • Harder to detect abuse if tests can be manipulated or if the update surface grows over time.

The check for modified files should address that. I don't expect the update surface to grow, it hasn't in a long time.

@flavorjones
Copy link
Author

I would like to see where the conversation in https://bugs.ruby-lang.org/issues/21804 goes before investing more time in this PR. If it's determined that auto-merging and auto-releasing is an acceptable path, I would be happy to make some of the changes suggested.

@ntkme
Copy link
Contributor

ntkme commented Jan 6, 2026

Another concern is that the bot PR does not update the test matrix when new major/minor ruby release is available, which can potentially lead to untested broken code getting released out. For example, I just found out that ruby 4.0 on windows is missing from test matrix: #853

Hypothetically, let's say ruby 4.0 on windows build was somehow broken, and the bot PR auto-merged the change because all tests were passing as the broken one wasn't even tested, for end-users who is using ruby-version: ruby to get the latest they can get a broken release due to auto-merge without proper test coverage.

This can be addressed either by letting the bot updating the workflow when necessary - this might be tricky given that we use matrix permutations with extra include/exclude. Alternatively, we can use a dynamic matrix - that is to use a job to generate the test matrix from the version files as JSON output, and then use dynamic value matrix: ${{ fromJSON: needs.prepare-matrix-job.outputs.matrix }}. This will allow us to automatically better cover what should be tested.

For example:

  prepare-matrix:
    runs-on: ubuntu-latest

    outputs:
      matrix: ${{ steps.matrix.outputs.matrix }}

    steps:
      - uses: actions/checkout@v6

      - id: matrix
        run: echo "matrix=$(ruby prepare-matrix-json.rb)" | tee -a "$GITHUB_OUTPUT"
        # prepare-matrix-json.rb would generate a JSON matrix one-liner from contents of
        # ruby-builder-versions.json and windows-versions.json

  test:
    needs: [prepare-matrix]

    runs-on: ubuntu-latest

    strategy:
      fail-fast: false
      matrix: ${{ fromJSON(needs.prepare-matrix.outputs.matrix) }}

@eregon
Copy link
Member

eregon commented Jan 6, 2026

Another concern is that the bot PR does not update the test matrix when new major/minor ruby release is available, which can potentially lead to untested broken code getting released out. For example, I just found out that ruby 4.0 on windows is missing from test matrix: #853

Good catch. I'll note however:

The generated test matrix idea sounds good, especially given that allowing these automated PRs to change the workflow would be rather dangerous.

@flavorjones
Copy link
Author

I would prefer if people commented on the ruby bug tracker (linked in the description) and not here about the general topic of auto-merging. Thanks.

@eregon
Copy link
Member

eregon commented Jan 8, 2026

FWIW https://bugs.ruby-lang.org is generally not an appropriate place to report issues related to setup-ruby, but I suppose in this case it's fine enough as it may concern work for other Ruby committers.
Given none of the release maintainers have replied though, I think we might already have our answer (automation).

@eregon
Copy link
Member

eregon commented Jan 8, 2026

I would prefer if people commented on the ruby bug tracker (linked in the description) and not here about the general topic of auto-merging. Thanks.

I reacted to this maybe rather strongly, I think the main reasons are:

  • For the last comment it's much better here and would feel out of place on https://bugs.ruby-lang.org/. In fact I think everything related to automation and technical aspects of it is better discussed here.
  • From my POV the discussion at https://bugs.ruby-lang.org/issues/21804 at this point is only about whether CRuby release managers want to help with this or not. Until they reply there is probably not much to discuss there.
  • I dislike the implicit pressure because CRuby is released at Christmas that OSS maintainers are expected to work during that time (I guess the only way to fix that would be to change the release schedule).
  • Unless some people are happy to work on Dec 25 and following days every single year, and are either release managers or setup-ruby maintainers, there won't be a true solution for people impatient to test the latest release in CI, besides automation.
  • The fact that e.g. native gems for sqlite3 and nokogiri are released during that period probably puts more pressure on dependencies to also do something, vs just waiting at every level. It's probably wishful thinking though that this could work.

@flavorjones
Copy link
Author

@eregon No worries. You're the maintainer and I appreciate your thoughts. I'll push some changes to this PR tonight to implement the additional checks you requested above.

@p-linnane
Copy link

  • I dislike the implicit pressure because CRuby is released at Christmas that OSS maintainers are expected to work during that time (I guess the only way to fix that would be to change the release schedule).

  • Unless some people are happy to work on Dec 25 and following days every single year, and are either release managers or setup-ruby maintainers, there won't be a true solution for people impatient to test the latest release in CI, besides automation.

It seems rather unlikely that CRuby’s release schedule will change.

This reads as a suggestion that because no current maintainers want to take this on, the solution is to remove the human element. Would it not make sense to instead add maintainers who are willing to help with this process, rather than removing human oversight altogether? If merging these PRs and cutting a release is considered so critical that it cannot reasonably be supported by expanding the maintainer group, that would seem to argue for keeping human review in place rather than automating it.

As an OSS maintainer myself, I am very aware that volunteer maintainers do not owe anything to anyone. When our maintainers are not interested in a particular task, we usually treat that as a signal to invite the broader community to step in and help fill the gap. I believe this argues for more human involvement in critical processes, not less.

@flavorjones
Copy link
Author

flavorjones commented Jan 9, 2026

As I mentioned in the description, I agree with @p-linnane. I am always working on the native gem / rake-compiler toolchain around the Ruby release date (and the toolchain and the dependent gems can't be fully tested without setup-ruby) so I would be happy to volunteer to fill a release manager role.

Per conversation in ruby#848, I've added automerge-check.rb which:

- checks changed files against a small allowlist
- checks the URLs in the windows version and toolchain files

The script contains tests for itself which can be run by passing a
`--test` flag on the commandline.

Note that automerge-check.rb is run from the base branch (master) to
avoid malicious PRs from changing the script.

Also note that dist/index.js is previously being checked in the "lint"
job in test.yml (which this pipeline depends upon).
@eregon
Copy link
Member

eregon commented Jan 9, 2026

Would it not make sense to instead add maintainers who are willing to help with this process, rather than removing human oversight altogether?

IMO maintainers of a project should (ideally/when possible) know the project well and e.g. have made several contributions before they become maintainers.
From that perspective I think @MSP-Greg and @ntkme for their significant contributions and help for setup-ruby certainly deserve to be maintainers, and I would be happy to add them, if they are interested.
They are not CRuby committers, but I would entrust them with this task (as said before, the blast radius is high so e.g. it seems not that great an idea to allow all CRuby committers to release this action, also if it's "all" nobody feels responsible).

However, I'm not sure it would solve this issue, in that they might also not e.g. check their email multiple times a day during the winter holiday break (which IMO is unreasonable to expect of anyone, people deserve holidays/a break).


For TruffleRuby, I took care to be able to merge a add-a-release PR for all projects that distribute it (ruby-versions, ruby-build, rvm, setup-ruby), so it's fast and reliable. This is even properly documented.

I think it would make sense for CRuby to do the same, i.e. that release managers (@hsbt, @nagachika and @k0kubun), because the release managers are typically available around the time of the release, also take care of these extra steps after the release is published:

  • Create a ruby-build PR and merge it (example, this need write permissions on that repo to merge PRs though)
  • Run this workflow
  • Wait for the automated setup-ruby PR, CI and merge it.
  • Run this workflow to create a setup-ruby release.

@hsbt @nagachika @k0kubun How about it?
And @headius, how about it for JRuby?

Regarding Windows, given the timing really depends on @larskanis, I think it would make most sense to ask him (and give him permission) to trigger this workflow, and then wait for the automated setup-ruby PR, CI and merge it and create a release.

So if they accept, we could give them write permission.
I would however not consider them "maintainers" of setup-ruby, more like "releasers" of setup-ruby, since e.g. I don't expect them to triage issues, i.e. it would be specifically about triggering the relevant workflows and merging such automated PRs.

From https://bugs.ruby-lang.org/issues/21804#note-10

Would CODEOWNERS or a ping from the bot when a PR is opened not suffice for the 'easy to forget' portion?

Yes, good idea, I think the automated PRs could just ping all these people in their description and also give a link for how to create a release, so it's all self-contained. Hopefully it doesn't amount to too much notifications, but given CRuby, JRuby and TruffleRuby release not very often it should be OK.

(BTW, I'm writing here because it's convenient and pings probably work better on GitHub than Redmine)

@eregon
Copy link
Member

eregon commented Jan 9, 2026

I consider access to https://github.com/ruby/ruby-builder to be very sensitive, so it'd be great there if we could use a custom role or a PAT or something to only allow triggering certain workflows but not e.g. modify release assets.
Maybe by having a workflow in this repo that uses a token to trigger that workflow, or so.

@MikeMcQuaid
Copy link

IMO maintainers of a project should (ideally/when possible) know the project well and e.g. have made several contributions before they become maintainers.

I would however not consider them "maintainers" of setup-ruby, more like "releasers" of setup-ruby, since e.g. I don't expect them to triage issues, i.e. it would be specifically about triggering the relevant workflows and merging such automated PRs.

My $0.02: on both of these: I would say my experience (from both sides of this) is it's worth having a lower bar for people who have a demonstrated track record of maintaining open source software operating at a scale of Ruby (or higher).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants