style: clear outstanding rubocop offences and lint in CI

.github/workflows/ci.yml

@@ -25,6 +25,21 @@ jobs:
 
       - run: bundle exec rake test
 
+  lint:
+    name: "Rubocop"
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: ruby/setup-ruby@v1
+        with:
+          # Matches TargetRubyVersion in .rubocop.yml.
+          ruby-version: "3.2"
+          bundler-cache: true
+
+      - run: bundle exec rubocop
+
   worker:
     name: "Worker"
     runs-on: ubuntu-latest

.rubocop.yml

@@ -12,6 +12,8 @@ Style/StringLiterals:
 
 Metrics/ClassLength:
   Max: 150
+  Exclude:
+    - "test/**/*"
 
 Metrics/AbcSize:
   Max: 30

lib/prebake/async_publisher.rb

@@ -76,7 +76,8 @@ def self.build_gem(spec, backend)
       end
 
       if !Prebake.libruby_available? && ElfInspector.libruby_needed_for_gem?(gem_path)
-        Logger.warn "Skipping push of #{cache_key}: binary requires libruby.so (dynamic Ruby) but this is a static Ruby build; binary would crash on this platform"
+        Logger.warn "Skipping push of #{cache_key}: binary requires libruby.so (dynamic Ruby) " \
+                    "but this is a static Ruby build; binary would crash on this platform"
         FileUtils.rm_f(gem_path)
         return nil
       end

lib/prebake/elf_inspector.rb

@@ -10,7 +10,10 @@ module Prebake
   module ElfInspector
     def self.required_glibc_for_gem(gem_path)
       versions = []
-      each_gem_binary(gem_path) { |binary| v = required_glibc(binary); versions << v if v }
+      each_gem_binary(gem_path) do |binary|
+        v = required_glibc(binary)
+        versions << v if v
+      end
       versions.empty? ? nil : versions.max_by { |v| Gem::Version.new(v) }
     rescue StandardError => e
       # Malformed gem, missing objdump, or I/O error — treat as unknown, let
@@ -69,7 +72,7 @@ def self.run_objdump(path)
       Dir.mktmpdir("prebake-gem") do |tmpdir|
         Gem::Package.new(gem_path).extract_files(tmpdir)
         Dir.glob(File.join(tmpdir, "**/*.{so,bundle,dll}")).each do |binary|
-          next if File.symlink?(binary) || File.size(binary).zero?
+          next if File.symlink?(binary) || File.empty?(binary)
 
           yield binary
         end

lib/prebake/ext_builder_patch.rb

@@ -11,6 +11,14 @@
 
 module Prebake
   module ExtBuilderPatch
+    # The metrics cops are off for this method deliberately. It is a cascade of
+    # cache checks where every failure path falls through to `super`, the real
+    # source build. Extracting any stage would mean returning a sentinel for the
+    # caller to turn back into `super`, which reads worse than the cascade and
+    # risks silently installing a gem with no compiled binaries if a branch is
+    # mistranslated.
+    # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
+    # rubocop:disable Metrics/MethodLength, Metrics/PerceivedComplexity
     def build_extensions
       return super unless @spec.extensions.any?
       return super unless Prebake.enabled?
@@ -59,14 +67,16 @@ def build_extensions
 
         unless installed
           Prebake.backend.delete(cache_key)
-          return super
+          super
         end
       else
         super
       end
     ensure
       FileUtils.rm_f(cached_gem) if cached_gem
     end
+    # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity
+    # rubocop:enable Metrics/MethodLength, Metrics/PerceivedComplexity
 
     private
 

lib/prebake/extension_validator.rb

@@ -24,7 +24,7 @@ def self.validate(spec)
 
       nested.each do |binary|
         next if File.symlink?(binary)
-        next if File.size(binary).zero?
+        next if File.empty?(binary)
 
         dest = File.join(ext_dir, File.basename(binary))
         next if File.exist?(dest)

lib/prebake/extractor.rb

@@ -17,35 +17,17 @@ def self.install(gem_path, spec)
 
         # Copy only binary files (.so, .bundle, .dll) to extension_dir
         Dir.glob(File.join(tmpdir, "**/*.{so,bundle,dll}")).each do |binary|
-          # Reject symlinks and empty files
-          next if File.symlink?(binary)
-          next if File.size(binary).zero?
-
-          # Verify path is within tmpdir (prevent traversal)
-          real_binary = File.realpath(binary)
-          real_tmpdir = File.realpath(tmpdir)
-          next unless real_binary.start_with?("#{real_tmpdir}/")
-
-          relative = binary.sub("#{tmpdir}/", "")
-
-          # Normalize paths from cached gems where binaries were packaged
-          # from gem_dir build artifacts or dirty extension_dirs.
-          # ext/<name>/<name>.so               → <name>.so       (build artifact)
-          # lib/<name>/<name>.so               → <name>/<name>.so (gem lib path)
-          # extension/<platform>/<ver>/<name>.so → <name>.so       (extension_dir artifact)
-          relative = relative.sub(%r{\Aext/[^/]+/}, "") if relative.start_with?("ext/")
-          relative = relative.sub(%r{\Alib/}, "") if relative.start_with?("lib/")
-          relative = relative.sub(%r{\Aextensions?/[^/]+/[^/]+/}, "") if relative.start_with?("extension/", "extensions/")
-
-          dest = File.join(spec.extension_dir, relative)
+          next unless safe_binary?(binary, tmpdir)
+
+          dest = File.join(spec.extension_dir, normalized_path(binary, tmpdir))
           FileUtils.mkdir_p(File.dirname(dest))
           FileUtils.cp(binary, dest)
           extracted_count += 1
         end
       end
 
       # Mark this extension_dir as prebake-managed for post-install validation
-      FileUtils.touch(File.join(spec.extension_dir, ".prebake")) if extracted_count > 0
+      FileUtils.touch(File.join(spec.extension_dir, ".prebake")) if extracted_count.positive?
 
       Logger.info "Installed precompiled #{File.basename(gem_path)} " \
                   "(#{extracted_count} binary files)"
@@ -55,5 +37,28 @@ def self.install(gem_path, spec)
       Logger.warn "Extraction failed for #{File.basename(gem_path)}: #{e.message}"
       raise
     end
+
+    # Reject symlinks, empty files, and anything resolving outside tmpdir
+    # (path traversal via a crafted gem).
+    def self.safe_binary?(binary, tmpdir)
+      return false if File.symlink?(binary)
+      return false if File.empty?(binary)
+
+      File.realpath(binary).start_with?("#{File.realpath(tmpdir)}/")
+    end
+
+    # Normalize paths from cached gems where binaries were packaged from
+    # gem_dir build artifacts or dirty extension_dirs.
+    # ext/<name>/<name>.so                 → <name>.so        (build artifact)
+    # lib/<name>/<name>.so                 → <name>/<name>.so (gem lib path)
+    # extension/<platform>/<ver>/<name>.so → <name>.so        (extension_dir artifact)
+    def self.normalized_path(binary, tmpdir)
+      relative = binary.sub("#{tmpdir}/", "")
+      relative = relative.sub(%r{\Aext/[^/]+/}, "") if relative.start_with?("ext/")
+      relative = relative.sub(%r{\Alib/}, "") if relative.start_with?("lib/")
+      return relative unless relative.start_with?("extension/", "extensions/")
+
+      relative.sub(%r{\Aextensions?/[^/]+/[^/]+/}, "")
+    end
   end
 end

lib/prebake/platform_gem_builder.rb

@@ -54,34 +54,7 @@ def build_platform_spec(build_dir)
       Dir.glob(File.join(build_dir, "**/*.{so,bundle,dll}")).each { |f| File.delete(f) }
 
       ext_dir = @spec.extension_dir
-      if ext_dir && File.directory?(ext_dir)
-        # Collect binaries at root and one level deep (e.g., nokogiri/nokogiri.so).
-        binaries = Dir.glob(File.join(ext_dir, "*.{so,bundle,dll}")) +
-                   Dir.glob(File.join(ext_dir, "*/*.{so,bundle,dll}"))
-
-        # Ruby 4.0+ places compiled extensions in extension/<platform>/<abi>/
-        # within extension_dir.  Collect these too, normalizing their paths
-        # to root level so the cached gem is layout-agnostic.
-        Dir.glob(File.join(ext_dir, "extension/*/*/*.{so,bundle,dll}")).each do |binary|
-          relative = binary.delete_prefix("#{ext_dir}/")
-          normalized = relative.sub(%r{\Aextension/[^/]+/[^/]+/}, "")
-          # Skip if a root-level binary with the same name already exists
-          next if binaries.any? { |b| b.delete_prefix("#{ext_dir}/") == normalized }
-
-          binaries << binary
-        end
-
-        binaries.each do |binary|
-          next if File.symlink?(binary)
-          next if File.size(binary).zero?
-          relative = binary.delete_prefix("#{ext_dir}/")
-          # Normalize extension/<platform>/<abi>/ paths to root level
-          relative = relative.sub(%r{\Aextension/[^/]+/[^/]+/}, "")
-          dest = File.join(build_dir, relative)
-          FileUtils.mkdir_p(File.dirname(dest))
-          FileUtils.cp(binary, dest)
-        end
-      end
+      copy_installed_binaries(ext_dir, build_dir) if ext_dir && File.directory?(ext_dir)
 
       prefix = "#{build_dir}/"
       compiled = Dir.glob(File.join(build_dir, "**/*.{so,bundle,dll}"))
@@ -90,5 +63,38 @@ def build_platform_spec(build_dir)
 
       platform_spec
     end
+
+    def copy_installed_binaries(ext_dir, build_dir)
+      installed_binaries(ext_dir).each do |binary|
+        next if File.symlink?(binary)
+        next if File.empty?(binary)
+
+        dest = File.join(build_dir, root_level_path(binary, ext_dir))
+        FileUtils.mkdir_p(File.dirname(dest))
+        FileUtils.cp(binary, dest)
+      end
+    end
+
+    # Binaries at root and one level deep (e.g., nokogiri/nokogiri.so), plus the
+    # extension/<platform>/<abi>/ layout Ruby 4.0+ installs into.  A nested
+    # binary is skipped when a root-level one already claims its normalized
+    # path, so the cached gem stays layout-agnostic.
+    def installed_binaries(ext_dir)
+      binaries = Dir.glob(File.join(ext_dir, "*.{so,bundle,dll}")) +
+                 Dir.glob(File.join(ext_dir, "*/*.{so,bundle,dll}"))
+
+      Dir.glob(File.join(ext_dir, "extension/*/*/*.{so,bundle,dll}")).each do |binary|
+        normalized = root_level_path(binary, ext_dir)
+        next if binaries.any? { |b| b.delete_prefix("#{ext_dir}/") == normalized }
+
+        binaries << binary
+      end
+
+      binaries
+    end
+
+    def root_level_path(binary, ext_dir)
+      binary.delete_prefix("#{ext_dir}/").sub(%r{\Aextension/[^/]+/[^/]+/}, "")
+    end
   end
 end

test/extension_validator_test.rb

@@ -27,7 +27,7 @@ def test_skips_without_marker
     Prebake::ExtensionValidator.validate(spec)
 
     refute File.exist?(File.join(@extension_dir, "foo.so")),
-      "Should not copy binaries when .prebake marker is absent"
+           "Should not copy binaries when .prebake marker is absent"
   end
 
   def test_skips_correct_layout
@@ -45,7 +45,7 @@ def test_skips_correct_layout
     Prebake::ExtensionValidator.validate(spec)
 
     refute File.exist?(File.join(@extension_dir, "bar.so")),
-      "Should not copy nested binaries when root-level binaries already exist"
+           "Should not copy nested binaries when root-level binaries already exist"
   end
 
   def test_fixes_extension_pattern
@@ -96,7 +96,7 @@ def test_preserves_nested_lib_subdirectory
     Prebake::ExtensionValidator.validate(spec)
 
     refute File.exist?(File.join(@extension_dir, "foo.so")),
-      "Should not copy binaries from lib/<subdir>/ (only direct children of lib/)"
+           "Should not copy binaries from lib/<subdir>/ (only direct children of lib/)"
   end
 
   def test_skips_symlinks
@@ -115,7 +115,7 @@ def test_skips_symlinks
     Prebake::ExtensionValidator.validate(spec)
 
     refute File.exist?(File.join(@extension_dir, "linked.so")),
-      "Should not copy symlinked binaries"
+           "Should not copy symlinked binaries"
   end
 
   def test_skips_empty_files
@@ -131,7 +131,7 @@ def test_skips_empty_files
     Prebake::ExtensionValidator.validate(spec)
 
     refute File.exist?(File.join(@extension_dir, "empty.so")),
-      "Should not copy zero-byte binaries"
+           "Should not copy zero-byte binaries"
   end
 
   def test_idempotent
@@ -155,5 +155,4 @@ def test_idempotent
     root_binaries = Dir.glob(File.join(@extension_dir, "*.so"))
     assert_equal 1, root_binaries.length, "Should not duplicate files on second run"
   end
-
 end