Hardening patches

.github/actions/cibuildgem/action.yml

@@ -22,21 +22,28 @@ runs:
   steps:
     - name: Install cibuildgem
       shell: bash
+      env:
+        CIBUILDGEM_VERSION: "${{ inputs.version }}"
       run: |-
         ${{ case(
           github.repository == 'Shopify/cibuildgem', 'rake install',
-          inputs.version != null, format('gem install cibuildgem -v {0}', inputs.version),
+          inputs.version != null, 'gem install cibuildgem -v "$CIBUILDGEM_VERSION"',
           'gem install cibuildgem'
         ) }}
       working-directory: "${{ github.action_path }}"
     - name: "Setup Rake Compiler"
       if: "${{ inputs.step == 'compile' }}"
       uses: actions/github-script@v8
+      env:
+        # Passed through the environment rather than interpolated into the script
+        # below: a value containing a quote would otherwise close the string
+        # literal and the remainder would be evaluated as JavaScript.
+        WORKING_DIRECTORY: "${{ inputs.working-directory }}"
       with:
         script: |
           const url = new URL(`file:///${process.env.GITHUB_ACTION_PATH}/dist/index.js`);
           const { run } = await import(url.href)
-          await run('${{ inputs.working-directory }}')
+          await run(process.env.WORKING_DIRECTORY)
     - name: Package
       if: "${{ inputs.step == 'compile' }}"
       working-directory: ${{ inputs.working-directory }}

CHANGELOG.md

@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- `cibuildgem release` no longer runs `gem push` through a shell. The `.gem` filenames it iterates over come from
+  artifacts built in the unprivileged compile job, and interpolating them into a command string let shell syntax in a
+  basename execute in the release job, after RubyGems credentials are configured. The filename is now passed as a
+  single argv element.
+- `cibuildgem release` now refuses a `.gem` whose basename holds anything other than letters, digits, dots, dashes
+  and underscores, rather than publishing it. RubyGems restricts a gem's name, version and platform to those
+  characters, so every filename `cibuildgem package` produces is accepted and one that is not was planted by
+  something else running in the compile job.
+- The action's `version` input is now passed to `gem install` through the step environment instead of being
+  interpolated into the shell script. A workflow that wired the input to a value it did not control turned a version
+  selector into arbitrary commands running in the job; the value now reaches `gem install` as a single argument and
+  RubyGems rejects it if it is not a version.
+
 ## [0.3.0] - 2026-03-27
 
 ### Added

lib/cibuildgem/cli.rb

@@ -11,6 +11,10 @@ class CLI < Thor
 
     source_root(File.expand_path("templates", __dir__))
 
+    # RubyGems already restricts a gem's name, version and platform to these characters, so every filename
+    # `cibuildgem package` can legitimately produce matches. Anything else was not produced by the package task.
+    RELEASABLE_GEM_FILENAME = /\A[A-Za-z0-9._-]+\.gem\z/
+
     class << self
       def exit_on_failure?
         true
@@ -113,7 +117,9 @@ def release
         pathname = Pathname(file)
         next if pathname.directory? || pathname.extname != ".gem"
 
-        out, status = Open3.capture2e("gem push #{file}")
+        verify_releasable_filename!(pathname)
+
+        out, status = Open3.capture2e("gem", "push", file)
         next if status.success?
 
         if out =~ /Repushing of gem versions is not allowed/
@@ -139,8 +145,20 @@ def print_normalized_platform
 
     private
 
+    def verify_releasable_filename!(pathname)
+      basename = pathname.basename.to_s
+      return if RELEASABLE_GEM_FILENAME.match?(basename)
+
+      raise(<<~MSG)
+        Refusing to publish #{basename.inspect}.
+
+        A gem filename may only contain letters, digits, dots, dashes and underscores, so this file was not
+        produced by `cibuildgem package`. Artifacts reach the release job from the compile job, which runs
+        without publishing credentials, so an unexpected filename is treated as tampering rather than a gem.
+      MSG
+    end
+
     def run_rake_tasks!(*tasks)
-      all_tasks = tasks.join(" ")
       rakelibdir = [File.expand_path("tasks", __dir__), "rakelib"].join(File::PATH_SEPARATOR)
       rake_compiler_path = Gem.loaded_specs["rake-compiler"].full_require_paths
       rake_specs = Gem.loaded_specs["rake"]
@@ -152,7 +170,14 @@ def run_rake_tasks!(*tasks)
 
       system(
         { "RUBYLIB" => load_paths },
-        "bundle exec #{RbConfig.ruby} #{rake_executable} #{all_tasks} -R#{rakelibdir} -r #{patch}",
+        "bundle",
+        "exec",
+        RbConfig.ruby,
+        rake_executable,
+        *tasks.map(&:to_s),
+        "-R#{rakelibdir}",
+        "-r",
+        patch,
         exception: true,
       )
     end

test/action_test.rb

@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "fileutils"
+require "tmpdir"
+require "yaml"
+
+module Cibuildgem
+  class ActionTest < Minitest::Test
+    ACTION_PATH = File.expand_path("../.github/actions/cibuildgem/action.yml", __dir__)
+
+    def test_github_script_steps_do_not_interpolate_workflow_expressions
+      github_script_steps.each do |step|
+        script = step.dig("with", "script")
+
+        refute_match(
+          /\$\{\{/,
+          script,
+          "Step #{step["name"].inspect} interpolates a workflow expression into evaluated " \
+            "JavaScript. Pass the value through `env:` and read it from `process.env` instead.",
+        )
+      end
+    end
+
+    def test_setup_rake_compiler_reads_the_working_directory_from_the_environment
+      step = github_script_steps.fetch(0)
+
+      assert_equal("${{ inputs.working-directory }}", step.dig("env", "WORKING_DIRECTORY"))
+      assert_includes(step.dig("with", "script"), "run(process.env.WORKING_DIRECTORY)")
+    end
+
+    def test_install_step_reads_the_cibuildgem_version_from_the_environment
+      assert_equal("${{ inputs.version }}", install_step.dig("env", "CIBUILDGEM_VERSION"))
+      assert_equal(%(gem install cibuildgem -v "$CIBUILDGEM_VERSION"), version_branch_script("1.2.3"))
+    end
+
+    def test_the_version_input_reaches_gem_install_as_a_single_argument
+      hostile_version = nil
+      argv = nil
+
+      Dir.mktmpdir do |dir|
+        marker = File.join(dir, "injected")
+        argv_log = File.join(dir, "argv")
+        hostile_version = "$(touch #{marker})"
+
+        system(
+          { "PATH" => "#{fake_gem_bin(dir, argv_log)}:#{ENV["PATH"]}", "CIBUILDGEM_VERSION" => hostile_version },
+          "bash",
+          "-c",
+          version_branch_script(hostile_version),
+          out: File::NULL,
+          err: File::NULL,
+        )
+        refute_path_exists(marker, "the version input was evaluated as shell syntax instead of passed as data")
+
+        argv = File.readlines(argv_log, chomp: true)
+      end
+
+      assert_equal(["install", "cibuildgem", "-v", hostile_version], argv)
+    end
+
+    private
+
+    def fake_gem_bin(dir, argv_log)
+      bin = File.join(dir, "bin")
+      FileUtils.mkdir_p(bin)
+      File.write(File.join(bin, "gem"), <<~SH)
+        #!/usr/bin/env bash
+        printf '%s\\n' "$@" > #{argv_log}
+      SH
+      FileUtils.chmod(0o755, File.join(bin, "gem"))
+
+      bin
+    end
+
+    def steps
+      YAML.safe_load_file(ACTION_PATH).dig("runs", "steps")
+    end
+
+    def install_step
+      steps.fetch(0).tap { |step| assert_equal("Install cibuildgem", step["name"]) }
+    end
+
+    # The `version` branch of the install step's `case()` expression, rendered as the shell script the runner
+    # would produce for `version`. Understands a literal command, which is the safe shape, and a `format()`
+    # call interpolating the input, which is the shape these tests exist to reject.
+    def version_branch_script(version)
+      branch = install_step.fetch("run")[/inputs\.version != null,\s*(.+?),\s*'[^']*'\s*\)/m, 1]
+      refute_nil(branch, "no `version` branch found in the install step")
+
+      case branch
+      when /\A'(.*)'\z/m then Regexp.last_match(1)
+      when /\Aformat\('(.*)',\s*inputs\.version\)\z/m then Regexp.last_match(1).sub("{0}", version)
+      else flunk("cannot tell what the install step runs when `version` is set: #{branch}")
+      end
+    end
+
+    def github_script_steps
+      matching = steps.select { |step| step["uses"].to_s.start_with?("actions/github-script") }
+
+      refute_empty(matching)
+      matching
+    end
+  end
+end

test/cli_test.rb

@@ -3,6 +3,7 @@
 require "test_helper"
 require "English"
 require "open3"
+require "tmpdir"
 
 module Cibuildgem
   class CLITest < Minitest::Test
@@ -137,8 +138,8 @@ def test_release_succeeds
 
       status = Struct.new(:success?)
       gem_pushed = []
-      callable = proc do |gem_name|
-        gem_pushed << gem_name
+      callable = proc do |*command|
+        gem_pushed << command
 
         ["", status.new(true)]
       end
@@ -147,7 +148,7 @@ def test_release_succeeds
         CLI.start(["release", "--glob", "tmp/*"])
       end
 
-      assert_equal(["gem push tmp/bar.gem", "gem push tmp/foo.gem"], gem_pushed.sort)
+      assert_equal([["gem", "push", "tmp/bar.gem"], ["gem", "push", "tmp/foo.gem"]], gem_pushed.sort)
     ensure
       FileUtils.rm_rf("tmp/foo.gem")
       FileUtils.rm_rf("tmp/bar.gem")
@@ -161,10 +162,10 @@ def test_release_when_gem_was_already_pushed
 
       status = Struct.new(:success?)
       gem_pushed = []
-      callable = proc do |gem_name|
-        gem_pushed << gem_name
+      callable = proc do |*command|
+        gem_pushed << command
 
-        if gem_name == "gem push tmp/bar.gem"
+        if command == ["gem", "push", "tmp/bar.gem"]
           ["Repushing of gem versions is not allowed", status.new(false)]
         else
           ["", status.new(true)]
@@ -179,7 +180,7 @@ def test_release_when_gem_was_already_pushed
         assert_equal("Gem tmp/bar.gem already exists on RubyGems.org, skipping...\n", out)
       end
 
-      assert_equal(["gem push tmp/bar.gem", "gem push tmp/foo.gem"], gem_pushed.sort)
+      assert_equal([["gem", "push", "tmp/bar.gem"], ["gem", "push", "tmp/foo.gem"]], gem_pushed.sort)
     ensure
       FileUtils.rm_rf("tmp/foo.gem")
       FileUtils.rm_rf("tmp/bar.gem")
@@ -207,6 +208,30 @@ def test_release_fails
       FileUtils.rm_rf("tmp/some_file")
     end
 
+    def test_release_refuses_a_filename_that_is_not_a_plain_gem_name
+      with_fake_gem_executable do |argv_log|
+        FileUtils.touch("pkg/hello$(touch injected).gem")
+
+        error = assert_raises(RuntimeError) do
+          CLI.start(["release", "--glob", "pkg/*"])
+        end
+
+        assert_match("hello$(touch injected).gem", error.message)
+        refute(File.exist?("injected"), "the filename was evaluated by a shell")
+        refute(File.exist?(argv_log), "the file was handed to `gem push` instead of being refused")
+      end
+    end
+
+    def test_release_pushes_a_plain_gem_name_without_a_shell
+      with_fake_gem_executable do |argv_log|
+        FileUtils.touch("pkg/hello_world-1.2.3-x86_64-linux.gem")
+
+        CLI.start(["release", "--glob", "pkg/*"])
+
+        assert_equal(["push", "pkg/hello_world-1.2.3-x86_64-linux.gem"], File.readlines(argv_log, chomp: true))
+      end
+    end
+
     def test_print_ruby_cc_version
       out, _ = capture_subprocess_io do
         Dir.chdir("test/fixtures/dummy_gem") do
@@ -315,8 +340,56 @@ def test_keep_the_extension_task_config_defined_by_the_gem
       end
     end
 
+    def test_run_rake_tasks_builds_an_argv_list_rather_than_a_command_string
+      cli = CLI.new
+      command = nil
+      recorder = ->(*args, **_kwargs) do
+        command = args
+
+        true
+      end
+
+      cli.stub(:system, recorder) do
+        cli.send(:run_rake_tasks!, "cibuildgem:setup", :compile)
+      end
+
+      _env, *argv = command
+
+      assert_equal(["bundle", "exec"], argv.first(2))
+      assert_includes(argv, "cibuildgem:setup")
+      assert_includes(argv, "compile")
+    end
+
     private
 
+    # Runs the block in a scratch directory holding an empty `pkg/`, with a `gem` on PATH that records its
+    # arguments in the file whose path is yielded. Nothing is stubbed, so the release path resolves and
+    # executes a real subprocess the way it does on a release runner.
+    def with_fake_gem_executable(&block)
+      original_path = ENV["PATH"]
+
+      Dir.mktmpdir do |dir|
+        argv_log = File.join(dir, "argv")
+        fake_bin = File.join(dir, "bin")
+
+        FileUtils.mkdir_p(fake_bin)
+        File.write(File.join(fake_bin, "gem"), <<~SH)
+          #!/bin/sh
+          printf '%s\\n' "$@" > #{argv_log}
+        SH
+        FileUtils.chmod(0o755, File.join(fake_bin, "gem"))
+        ENV["PATH"] = [fake_bin, original_path].join(File::PATH_SEPARATOR)
+
+        Dir.chdir(dir) do
+          FileUtils.mkdir_p("pkg")
+
+          block.call(argv_log)
+        end
+      end
+    ensure
+      ENV["PATH"] = original_path
+    end
+
     def raise_instead_of_exit(&block)
       Kernel.stub(:exit, ->(_) { raise }) do
         assert_raises(StandardError) do