[rubygems/rubygems] Converts Bundler lockfile checksum validation to …

…opt-in only

Looks for the CHECKSUMS section in the lockfile, activating the feature
only if the section exists. Without a CHECKSUMS section, Bundler will
continue as normal, validating checksums when gems are installed while
checksums from the compact index are present.

rubygems/rubygems@2353cc93a4

lib/bundler/checksum.rb

@@ -9,6 +9,18 @@ class Checksum
     private_constant :DEFAULT_BLOCK_SIZE
 
     class << self
+      def from_gem_package(gem_package, algo = DEFAULT_ALGORITHM)
+        return if Bundler.settings[:disable_checksum_validation]
+        return unless source = gem_package.instance_variable_get(:@gem)
+        return unless source.respond_to?(:with_read_io)
+
+        source.with_read_io do |io|
+          from_gem(io, source.path)
+        ensure
+          io.rewind
+        end
+      end
+
       def from_gem(io, pathname, algo = DEFAULT_ALGORITHM)
         digest = Bundler::SharedHelpers.digest(algo.upcase).new
         buf = String.new(:capacity => DEFAULT_BLOCK_SIZE)
@@ -17,6 +29,7 @@ def from_gem(io, pathname, algo = DEFAULT_ALGORITHM)
       end
 
       def from_api(digest, source_uri, algo = DEFAULT_ALGORITHM)
+        return if Bundler.settings[:disable_checksum_validation]
         Checksum.new(algo, to_hexdigest(digest, algo), Source.new(:api, source_uri))
       end
 
@@ -177,7 +190,6 @@ def fetch(spec, algo = DEFAULT_ALGORITHM)
       # This ensures a mismatch error where there are multiple top level sources
       # that contain the same gem with different checksums.
       def replace(spec, checksum)
-        return if Bundler.settings[:disable_checksum_validation]
         return unless checksum
 
         name_tuple = spec.name_tuple
@@ -193,7 +205,6 @@ def replace(spec, checksum)
       end
 
       def register(spec, checksum)
-        return if Bundler.settings[:disable_checksum_validation]
         return unless checksum
         register_checksum(spec.name_tuple, checksum)
       end

lib/bundler/definition.rb

@@ -18,7 +18,8 @@ class << self
       :platforms,
       :ruby_version,
       :lockfile,
-      :gemfiles
+      :gemfiles,
+      :locked_checksums
     )
 
     # Given a gemfile and lockfile creates a Bundler definition
@@ -92,6 +93,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
         @locked_bundler_version = @locked_gems.bundler_version
         @locked_ruby_version = @locked_gems.ruby_version
         @originally_locked_specs = SpecSet.new(@locked_gems.specs)
+        @locked_checksums = @locked_gems.checksums
 
         if unlock != true
           @locked_deps    = @locked_gems.dependencies
@@ -112,6 +114,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
         @originally_locked_specs = @locked_specs
         @locked_sources = []
         @locked_platforms = []
+        @locked_checksums = nil
       end
 
       locked_gem_sources = @locked_sources.select {|s| s.is_a?(Source::Rubygems) }
@@ -767,7 +770,7 @@ def converge_sources
       sources.all_sources.each do |source|
         # has to be done separately, because we want to keep the locked checksum
         # store for a source, even when doing a full update
-        if @locked_gems && locked_source = @locked_gems.sources.find {|s| s == source && !s.equal?(source) }
+        if @locked_checksums && @locked_gems && locked_source = @locked_gems.sources.find {|s| s == source && !s.equal?(source) }
           source.checksum_store.merge!(locked_source.checksum_store)
         end
         # If the source is unlockable and the current command allows an unlock of

lib/bundler/endpoint_specification.rb

@@ -125,7 +125,6 @@ def parse_metadata(data)
         next unless v
         case k.to_s
         when "checksum"
-          next if Bundler.settings[:disable_checksum_validation]
           begin
             @checksum = Checksum.from_api(v.last, @spec_fetcher.uri)
           rescue ArgumentError => e

lib/bundler/lockfile_generator.rb

@@ -67,6 +67,7 @@ def add_dependencies
     end
 
     def add_checksums
+      return unless definition.locked_checksums
       checksums = definition.resolve.map do |spec|
         spec.source.checksum_store.to_lock(spec)
       end

lib/bundler/lockfile_parser.rb

@@ -24,7 +24,15 @@ def to_s
       end
     end
 
-    attr_reader :sources, :dependencies, :specs, :platforms, :bundler_version, :ruby_version, :checksums
+    attr_reader(
+      :sources,
+      :dependencies,
+      :specs,
+      :platforms,
+      :bundler_version,
+      :ruby_version,
+      :checksums,
+    )
 
     BUNDLED      = "BUNDLED WITH"
     DEPENDENCIES = "DEPENDENCIES"
@@ -111,6 +119,9 @@ def initialize(lockfile)
         elsif line == DEPENDENCIES
           @parse_method = :parse_dependency
         elsif line == CHECKSUMS
+          # This is a temporary solution to make this feature disabled by default
+          # for all gemfiles that don't already explicitly include the feature.
+          @checksums = true
           @parse_method = :parse_checksum
         elsif line == PLATFORMS
           @parse_method = :parse_platform
@@ -228,8 +239,6 @@ def parse_checksum(line)
       version = Gem::Version.new(version)
       platform = platform ? Gem::Platform.new(platform) : Gem::Platform::RUBY
       full_name = Gem::NameTuple.new(name, version, platform).full_name
-      # Don't raise exception if there's a checksum for a gem that's not in the lockfile,
-      # we prefer to heal invalid lockfiles
       return unless spec = @specs[full_name]
 
       checksums.split(",") do |lock_checksum|

lib/bundler/rubygems_gem_installer.rb

@@ -103,15 +103,7 @@ def spec
     end
 
     def gem_checksum
-      return nil if Bundler.settings[:disable_checksum_validation]
-      return nil unless source = @package.instance_variable_get(:@gem)
-      return nil unless source.respond_to?(:with_read_io)
-
-      source.with_read_io do |io|
-        Checksum.from_gem(io, source.path)
-      ensure
-        io.rewind
-      end
+      Checksum.from_gem_package(@package)
     end
 
     private

spec/bundler/bundler/definition_spec.rb

@@ -56,6 +56,11 @@
         s.add_dependency "rack", "1.0"
       end
 
+      checksums = checksums_section_when_existing do |c|
+        c.no_checksum "foo", "1.0"
+        c.checksum gem_repo1, "rack", "1.0.0"
+      end
+
       bundle :install, :env => { "DEBUG" => "1" }
 
       expect(out).to match(/re-resolving dependencies/)
@@ -76,11 +81,7 @@
 
         DEPENDENCIES
           foo!
-
-        CHECKSUMS
-          #{gem_no_checksum "foo", "1.0"}
-          #{checksum_for_repo_gem gem_repo1, "rack", "1.0.0"}
-
+        #{checksums}
         BUNDLED WITH
            #{Bundler::VERSION}
       G
@@ -110,6 +111,11 @@
         s.add_development_dependency "net-ssh", "1.0"
       end
 
+      checksums = checksums_section_when_existing do |c|
+        c.no_checksum "foo", "1.0"
+        c.checksum gem_repo1, "rack", "1.0.0"
+      end
+
       install_gemfile <<-G
         source "#{file_uri_for(gem_repo1)}"
         gem "foo", :path => "#{lib_path("foo")}"
@@ -135,17 +141,17 @@
 
         DEPENDENCIES
           foo!
-
-        CHECKSUMS
-          #{gem_no_checksum "foo", "1.0"}
-          #{checksum_for_repo_gem gem_repo1, "rack", "1.0.0"}
-
+        #{checksums}
         BUNDLED WITH
            #{Bundler::VERSION}
       G
     end
 
     it "for a locked gem for another platform" do
+      checksums = checksums_section_when_existing do |c|
+        c.no_checksum "only_java", "1.1", "java"
+      end
+
       install_gemfile <<-G
         source "#{file_uri_for(gem_repo1)}"
         gem "only_java", platform: :jruby
@@ -166,16 +172,17 @@
 
         DEPENDENCIES
           only_java
-
-        CHECKSUMS
-          only_java (1.1-java)
-
+        #{checksums}
         BUNDLED WITH
            #{Bundler::VERSION}
       G
     end
 
     it "for a rubygems gem" do
+      checksums = checksums_section_when_existing do |c|
+        c.checksum gem_repo1, "foo", "1.0"
+      end
+
       install_gemfile <<-G
         source "#{file_uri_for(gem_repo1)}"
         gem "foo"
@@ -195,10 +202,7 @@
 
         DEPENDENCIES
           foo
-
-        CHECKSUMS
-          #{checksum_for_repo_gem gem_repo1, "foo", "1.0"}
-
+        #{checksums}
         BUNDLED WITH
            #{Bundler::VERSION}
       G

spec/bundler/cache/gems_spec.rb

@@ -289,11 +289,24 @@
       expect(cached_gem("rack-1.0.0")).to exist
     end
 
-    it "raises an error when the gem file is altered and produces a different checksum" do
+    it "raises an error when the gem is altered and produces a different checksum" do
       cached_gem("rack-1.0.0").rmtree
       build_gem "rack", "1.0.0", :path => bundled_app("vendor/cache")
+
+      checksums = checksums_section do |c|
+        c.checksum gem_repo1, "rack", "1.0.0"
+      end
+
       simulate_new_machine
 
+      lockfile <<-L
+        GEM
+          remote: #{file_uri_for(gem_repo2)}/
+          specs:
+            rack (1.0.0)
+        #{checksums}
+      L
+
       bundle :install, :raise_on_error => false
       expect(exitstatus).to eq(37)
       expect(err).to include("Bundler found mismatched checksums.")
@@ -305,6 +318,22 @@
       expect(cached_gem("rack-1.0.0")).to exist
     end
 
+    it "installs a modified gem with a non-matching checksum when checksums is not opted in" do
+      cached_gem("rack-1.0.0").rmtree
+      build_gem "rack", "1.0.0", :path => bundled_app("vendor/cache")
+      simulate_new_machine
+
+      lockfile <<-L
+        GEM
+          remote: #{file_uri_for(gem_repo2)}/
+          specs:
+            rack (1.0.0)
+      L
+
+      bundle :install
+      expect(cached_gem("rack-1.0.0")).to exist
+    end
+
     it "handles directories and non .gem files in the cache" do
       bundled_app("vendor/cache/foo").mkdir
       File.open(bundled_app("vendor/cache/bar"), "w") {|f| f.write("not a gem") }

spec/bundler/commands/check_spec.rb

@@ -406,6 +406,12 @@
     it "returns success when the Gemfile is satisfied and generates a correct lockfile" do
       system_gems "depends_on_rack-1.0", "rack-1.0", :gem_repo => gem_repo4, :path => default_bundle_path
       bundle :check
+
+      checksums = checksums_section_when_existing do |c|
+        c.no_checksum "depends_on_rack", "1.0"
+        c.no_checksum "rack", "1.0"
+      end
+
       expect(out).to include("The Gemfile's dependencies are satisfied")
       expect(lockfile).to eq <<~L
         GEM
@@ -424,11 +430,7 @@
 
         DEPENDENCIES
           depends_on_rack!
-
-        CHECKSUMS
-          depends_on_rack (1.0)
-          rack (1.0)
-
+        #{checksums}
         BUNDLED WITH
            #{Bundler::VERSION}
       L
@@ -468,6 +470,12 @@
 
       bundle "check --verbose", :dir => tmp.join("bundle-check-issue")
 
+      checksums = checksums_section_when_existing do |c|
+        c.checksum gem_repo4, "awesome_print", "1.0"
+        c.no_checksum "bundle-check-issue", "9999"
+        c.checksum gem_repo2, "dex-dispatch-engine", "1.0"
+      end
+
       expect(File.read(tmp.join("bundle-check-issue/Gemfile.lock"))).to eq <<~L
         PATH
           remote: .
@@ -491,12 +499,7 @@
         DEPENDENCIES
           bundle-check-issue!
           dex-dispatch-engine!
-
-        CHECKSUMS
-          #{checksum_for_repo_gem gem_repo4, "awesome_print", "1.0"}
-          bundle-check-issue (9999)
-          #{checksum_for_repo_gem gem_repo2, "dex-dispatch-engine", "1.0"}
-
+        #{checksums}
         BUNDLED WITH
            #{Bundler::VERSION}
       L