Auto-merge ruby-builder-bot PRs

[flavorjones]

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.

[p-linnane]

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.

[eregon]

Suggested change

	--squash \
	--rebase \

Because I always use Rebase and merge in this repo

[eregon]

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]

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]

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]

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]

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:

• Ruby itself is already tested upstream (in ruby/ruby CI and in RubyInstaller builds for Windows)
• There are some tests at https://github.com/ruby/ruby-builder/blob/07f4745e8eae1cc2b717dd2708fa0ee17778f9bf/.github/workflows/build.yml#L107-L114 for non-Windows, which is a subset of the tests done in this repo.
• If there was an issue with a RubyInstaller build the only thing we can do is report it, and we'd probably still add the version.
• If there is an issue in setup-ruby code for that version, then that could be missed and cause issues, that's the problematic one.

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

[flavorjones]

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]

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]

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]

@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]

• 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]

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.

[p-linnane]

Thanks for offering that, @flavorjones. I’d also be happy to help if additional maintainer or release support would be useful.

[ntkme]

Url need to be canonicalized before prefix checking as @eregon mentioned earlier - there is a risk of URL injection. For example a bad actor can put a URL like:

https://github.com/oneclick/rubyinstaller/releases/download/../../../../hacker/repo/releases/download/compromised-ruby.tar.gz

Or a bit more sneaky (depends on the client/server implementation this injection may or may not work):

https://github.com/oneclick/rubyinstaller/releases/download/%2E%2E/%2E%2E/%2E%2E/%2E%2E/hacker/repo/releases/download/compromised-ruby.tar.gz

As far as I know the built-in ruby uri gem does not have the capability of fully normalize the url path. On other hand, node.js URL class does path normalization out of box.

One solution in ruby without extra dependency is to use regex to match full url with limited use of wild card so that injection is impossible:

url = 'https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-4.0.0-1/rubyinstaller-4.0.0-1-arm.7z'
r = %r{https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-[^/%]+/rubyinstaller-[^/%]+\.7z}

r =~ url # => 0