From c29a8a184787415c6fc049187bdaf76e392a5d1a Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:45:29 +0800 Subject: [PATCH] fix: further isolate bundled jars/wars from wider environment Currently there are many gotchas with warbler's bundling that require user awareness. We can do much better to "do the right thing" by defualt, by setting opinionated bundler config that is baked into the warbled application. This change - bakes in a `.bundle/config` to the application, rather than relying on env vars, to ensure the strongest level of enforcement - ensures the following settings - `BUNDLE_VERSION = system` - ensure that bundler never tries to switch versions; and clarify that bundler will always run from the given jruby version's bundler. - `BUNDLE_FROZEN = true` - invert the previous default. Frozen should be the default, for both security and correctness. Users can still opt-out, if we have missed some case. - `BUNDLE_PATH__SYSTEM = true` - not strictly necessary, but clarifies that the jars/wars Warbler creates put all the gems onto what is effectively the system path at runtime. - `BUNDLE_AUTO_INSTALL = false` - enforce that installation of gems at runtime is impossible. Doubt anyone opts-in for this, but clarify that it won't work, and goes against the design goals of warbler. Also cleans things up related bugs for modern bundler - properly excludes default gem stubs from being included, which for bundler can cause `CorruptBundlerInstallError` if there is any version mismatch. - avoid duplicate gems when bundling relative path dependencies (#465) - remove unnecessary legacy `:warbler_excluded` workaround for bundler issue handling excluded git specs (from #42) - correct support for `gem_excludes` within bundler git specs. (#331) --- CHANGELOG.md | 3 + lib/warbler/config.rb | 6 +- lib/warbler/jar.rb | 2 +- lib/warbler/templates/bundler.erb | 3 - lib/warbler/traits/bundler.rb | 117 ++++++++++++++--------- spec/warbler/bundler_spec.rb | 150 +++++++++++++++++++++++++++--- 6 files changed, 219 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ec9bc4d..dfc25be2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## 2.1.2 +- #649: fix: further improve isolation of warbled jars/wars from host bundler configuration - #648: fix: further improve isolation of warbled jars/wars from user/host gem paths +- #465: fix: remove duplication of bundler path gems in warbled jars/wars +- #331: fix: correct support for `gem_excludes` inside bundler git specs - chore: relax jruby-rack requirement to allow compatibility with upcoming 2.0.x ## 2.1.1 diff --git a/lib/warbler/config.rb b/lib/warbler/config.rb index e3a9f2a1..bf59074f 100644 --- a/lib/warbler/config.rb +++ b/lib/warbler/config.rb @@ -60,8 +60,10 @@ class Config # Whether to include dependent gems (default true) attr_accessor :gem_dependencies - # Array of regular expressions matching relative paths in gems to - # be excluded from the war. Default contains no exclusions. + # Array of regular expressions matching paths *inside* packed gems to be + # excluded from the archive. Paths are matched relative to each gem's root + # (for git-sourced gems: the repository checkout root). Default contains no + # exclusions. attr_accessor :gem_excludes # Whether to exclude **/*.log files (default is true) diff --git a/lib/warbler/jar.rb b/lib/warbler/jar.rb index f0649637..f7d171c8 100644 --- a/lib/warbler/jar.rb +++ b/lib/warbler/jar.rb @@ -219,7 +219,7 @@ def find_single_gem_files(config, spec) # OK if the gem does not exists as its un-packed on the "shared" path # ... at least gem spec.spec_file should exists although not crucial if JRUBY_VERSION != JRubyJars::VERSION - warn "skipping #{spec.name} default gem (assuming its part of jruby-jars #{JRubyJars::VERSION})" unless silent? + warn "skipping #{spec.name} default gem (assuming its part of jruby-jars #{JRubyJars::VERSION})" end else warn "skipping #{spec.name} gem (#{full_gem_path.to_s} does not exist)" diff --git a/lib/warbler/templates/bundler.erb b/lib/warbler/templates/bundler.erb index 2f49dcc0..35c5a1ec 100644 --- a/lib/warbler/templates/bundler.erb +++ b/lib/warbler/templates/bundler.erb @@ -1,4 +1 @@ ENV['BUNDLE_WITHOUT'] = '<%= config.bundle_without.join(':') %>' -<% if config.bundler[:frozen] -%> -ENV['BUNDLE_FROZEN'] = '1' -<% end -%> diff --git a/lib/warbler/traits/bundler.rb b/lib/warbler/traits/bundler.rb index 22e482a5..e3f077ca 100644 --- a/lib/warbler/traits/bundler.rb +++ b/lib/warbler/traits/bundler.rb @@ -14,6 +14,23 @@ class Bundler include PathmapHelper include BundlerHelper + # Bundler settings enforced in the packed application via a generated + # *.bundle/config*, the highest-precedence documented configuration level: + BUNDLE_CONFIG_DEFAULTS = { + # BUNDLE_VERSION: disable bundler's "auto-switch" to Gemfile.lock's BUNDLED WITH bundler version + # (bundler restarts the process to do so, which cannot work inside a servlet container or self-contained executable jar) + 'BUNDLE_VERSION' => 'system', + # BUNDLE_FROZEN: fail fast with a descriptive error on Gemfile vs Gemfile.lock drift instead of attempting + # a runtime re-resolution + 'BUNDLE_FROZEN' => 'true', + # BUNDLE_PATH__SYSTEM (path.system): pin bundler to the "system" gems - which are the packed gems, via + # GEM_HOME/GEM_PATH from *init.rb* - immune to a BUNDLE_PATH/BUNDLE_DEPLOYMENT leaking in from the host's + # environment or ~/.bundle/config + 'BUNDLE_PATH__SYSTEM' => 'true', + # BUNDLE_AUTO_INSTALL: never install gems at boot time, even if the host's environment or global config enables it + 'BUNDLE_AUTO_INSTALL' => 'false' + }.freeze + def self.detect? File.exist?(ENV['BUNDLE_GEMFILE'] || 'Gemfile') end @@ -36,6 +53,8 @@ def add_bundler_gems; require 'bundler' config.gem_dependencies = false # Bundler takes care of these config.bundler = {} if config.bundler == true + warn_on_bundler_version_mismatch + bundler_specs.each do |spec| spec = to_spec(spec) @@ -45,11 +64,12 @@ def add_bundler_gems; require 'bundler' config.bundler[:git_specs] << spec when ::Bundler::Source::Path unless bundler_source_is_warbled_gem_itself?(spec.source) - if spec.source.path && spec.source.path.relative? - # include (assuming) relative *[APP_ROOT]/gem/path* - # NOTE: might be tuned to only add gemspec.files ... + if spec.source.path&.relative? + # pack the gem at its relative *[APP_ROOT]/gem/path* location - at runtime bundler resolves the path + # source relative to the packed Gemfile. + # Deliberately NOT also added to config.gems: bundler never materializes path gems from the gem + # repository; doing so would cause duplication. config.includes += FileList[File.join(spec.source.path, '**/*')] - config.gems << spec # probably not really needed else warn("Bundler `path' components are not fully supported.\n" + "The `#{spec.full_name}' component was not bundled.\n" + @@ -57,13 +77,12 @@ def add_bundler_gems; require 'bundler' end end else - config.gems << spec + config.gems << spec unless spec.respond_to?(:default_gem?) && spec.default_gem? end end config.bundler[:gemfile] = ::Bundler.default_gemfile config.bundler[:gemfile_path] = apply_pathmaps(config, relative_from_pwd(::Bundler.default_gemfile), :application) config.bundler[:lockfile] = ::Bundler.default_lockfile - config.bundler[:frozen] = ::Bundler.settings[:frozen] path = ::Bundler.settings[:path] config.excludes += [path, "#{path}/**/*"] if path config.init_contents << "#{config.warbler_templates}/bundler.erb" @@ -73,42 +92,41 @@ def update_archive(jar) add_bundler_files(jar) if config.bundler end - # Add Bundler Gemfiles and git repositories to the archive. + # Add Bundler Gemfiles, .bundle/config and git repositories to the archive. def add_bundler_files(jar) gemfile = relative_from_pwd(config.bundler[:gemfile]) lockfile = relative_from_pwd(config.bundler[:lockfile]) + bundle_config = File.join('.bundle', 'config') + jar.files[apply_pathmaps(config, gemfile, :application)] = config.bundler[:gemfile].to_s - if File.exist?(lockfile) - jar.files[apply_pathmaps(config, lockfile, :application)] = config.bundler[:lockfile].to_s - end - if config.bundler[:git_specs] - pathmap = "#{config.relative_gem_path}/bundler/gems/%p" - pathmap.sub!(%r{^/+}, '') - config.pathmaps.git = [pathmap] - config.bundler[:git_specs].each do |spec| - full_gem_path = Pathname.new(spec.full_gem_path) - - gem_relative_path = full_gem_path.relative_path_from(::Bundler.install_path) - filenames = [] - gem_relative_path.each_filename { |f| filenames << f } - - exclude_gems = true - unless filenames.empty? - full_gem_path = Pathname.new(::Bundler.install_path) + filenames.first - exclude_gems = false - end + jar.files[apply_pathmaps(config, lockfile, :application)] = config.bundler[:lockfile].to_s if File.exist?(lockfile) + # NOTE: in-memory content must be an IO - jar creation treats plain Strings as source file paths + jar.files[apply_pathmaps(config, bundle_config, :application)] = StringIO.new(bundle_config_contents) - if spec.groups.include?(:warbler_excluded) - pattern = "#{full_gem_path.to_s}/**/#{spec.name}.gemspec" # #42: gemspec only to avert Bundler error - else - pattern = "#{full_gem_path.to_s}/**/*" - end + add_bundler_git_specs(config.bundler[:git_specs], jar) if config.bundler[:git_specs] + end - FileList[pattern].each do |src| - f = Pathname.new(src).relative_path_from(full_gem_path).to_s - next if exclude_gems && config.gem_excludes && config.gem_excludes.any? {|rx| f =~ rx } - jar.files[apply_pathmaps(config, File.join(full_gem_path.basename, f), :git)] = src - end + private + + def add_bundler_git_specs(git_specs, jar) + config.pathmaps.git = ["#{config.relative_gem_path}/bundler/gems/%p".sub(%r{^/+}, '')] + + # a git source checkout may contain multiple gems (spec.full_gem_path being a sub-directory) - bundler expects + # the complete repository checkout under bundler/gems/-, so pack from its root (once per repository, + # even when several specs share the checkout) + checkout_paths = git_specs.map do |spec| + full_gem_path = Pathname.new(spec.full_gem_path) + filenames = full_gem_path.relative_path_from(::Bundler.install_path).each_filename.to_a + filenames.empty? ? full_gem_path : Pathname.new(::Bundler.install_path) + filenames.first + end.uniq + + checkout_paths.each do |checkout_path| + FileList["#{checkout_path.to_s}/**/*"].each do |src| + f = Pathname.new(src).relative_path_from(checkout_path).to_s + # NOTE: for git sources the excludes match relative to the packed repository checkout root (the gem root, + # except multi-gem repos) + next if config.gem_excludes && config.gem_excludes.any? { |rx| f =~ rx } + jar.files[apply_pathmaps(config, File.join(checkout_path.basename, f), :git)] = src end end end @@ -121,17 +139,32 @@ def relative_from_pwd(path) end end - private + def warn_on_bundler_version_mismatch + lockfile = ::Bundler.default_lockfile + return unless lockfile && File.exist?(lockfile) + locked = ::Bundler::LockfileParser.new(File.read(lockfile)).bundler_version rescue nil + if locked && locked.to_s != ::Bundler::VERSION + warn("Gemfile.lock BUNDLED WITH (#{locked}) does not match the bundler running warbler (#{::Bundler::VERSION}).\n" + + "The packed application will boot with the default bundler of the packed JRuby (jruby-jars),\n" + + "consider re-generating Gemfile.lock with a matching bundler version.") + end + end + + # Contents for the packed *.bundle/config*: only warbler's deployment settings, deliberately independent of the + # application's build-time bundler configuration. + def bundle_config_contents + require 'yaml' + settings = BUNDLE_CONFIG_DEFAULTS.dup + # frozen mode errors without a lockfile - do not force it upon applications packed without a Gemfile.lock + settings.delete('BUNDLE_FROZEN') if config.bundler[:frozen] == false || !File.exist?(config.bundler[:lockfile].to_s) + settings.to_yaml + end def bundler_specs bundle_without = config.bundle_without.map { |s| s.to_sym } definition = ::Bundler.definition - all = definition.specs.to_a requested_groups = definition.groups - bundle_without - requested = requested_groups.empty? ? [] : definition.specs_for(requested_groups).to_a - excluded_git_specs = (all - requested).select { |spec| ::Bundler::Source::Git === spec.source } - excluded_git_specs.each { |spec| spec.groups << :warbler_excluded } - requested + excluded_git_specs + requested_groups.empty? ? [] : definition.specs_for(requested_groups).to_a end def bundler_source_is_warbled_gem_itself?(source) diff --git a/spec/warbler/bundler_spec.rb b/spec/warbler/bundler_spec.rb index 58ef5a0c..f978d927 100644 --- a/spec/warbler/bundler_spec.rb +++ b/spec/warbler/bundler_spec.rb @@ -8,6 +8,7 @@ require File.expand_path('../../spec_helper', __FILE__) require 'open3' require 'bundler' +require 'yaml' describe Warbler::Jar, "with Bundler" do use_fresh_rake_application @@ -66,6 +67,67 @@ def apply_silently expect(file_list(%r{WEB-INF/Gemfile.lock})).to_not be_empty end + it "generates an opinionated .bundle/config into the war" do + File.open("Gemfile.lock", "w") {|f| f << "GEM"} + apply_silently + expect(file_list(%r{WEB-INF/\.bundle/config})).to_not be_empty + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings['BUNDLE_VERSION']).to eq 'system' + expect(settings['BUNDLE_FROZEN']).to eq 'true' + expect(settings['BUNDLE_PATH__SYSTEM']).to eq 'true' + expect(settings['BUNDLE_AUTO_INSTALL']).to eq 'false' + end + + it "does not force BUNDLE_FROZEN without a Gemfile.lock" do + apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings).to_not have_key 'BUNDLE_FROZEN' + expect(settings['BUNDLE_VERSION']).to eq 'system' + end + + it "does not force BUNDLE_FROZEN when opted out via config.bundler[:frozen]" do + File.open("Gemfile.lock", "w") {|f| f << "GEM"} + use_config do |config| + config.bundler = { :frozen => false } + end + apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings).to_not have_key 'BUNDLE_FROZEN' + expect(settings['BUNDLE_VERSION']).to eq 'system' # other deployment settings unaffected + end + + context "with an application-provided .bundle/config" do + before :each do + FileUtils.mkdir_p('.bundle') + File.write File.join('.bundle', 'config'), <<-CONFIG.gsub(/^ {10}/, '') + --- + BUNDLE_RETRY: "5" + BUNDLE_VERSION: "lockfile" + BUNDLE_GITHUB__COM: "user:secret" + CONFIG + end + + after(:each) { FileUtils.rm_rf('.bundle') } + + it "does not carry any of it into the archive (only warbler's deployment settings)" do + File.open("Gemfile.lock", "w") {|f| f << "GEM"} + apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings).to_not have_key 'BUNDLE_RETRY' + expect(settings).to_not have_key 'BUNDLE_GITHUB__COM' # credentials never packed + expect(settings['BUNDLE_VERSION']).to eq 'system' + expect(settings.keys).to match_array %w(BUNDLE_VERSION BUNDLE_FROZEN BUNDLE_PATH__SYSTEM BUNDLE_AUTO_INSTALL) + end + end + + it "does not package default gems (provided by the JRuby runtime)" do + File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rspec'\ngem 'stringio'"} + apply_silently + expect(file_list(%r{WEB-INF/gems/specifications/rspec})).to_not be_empty + expect(file_list(%r{WEB-INF/gems/specifications/stringio})).to be_empty + expect(file_list(%r{WEB-INF/gems/gems/stringio})).to be_empty + end + it "allows overriding of the gem path when using Bundler" do use_config do |config| config.gem_path = '/WEB-INF/jewels' @@ -85,14 +147,25 @@ def apply_silently expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/tester.gemspec})).to_not be_empty end - it "bundles only the gemspec for :git entries that are excluded" do - File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rake'\ngroup :test do\ngem 'tester', :git => '#{@gem_dir}'\nend\n"} + it "respects config.gem_excludes for :git entries" do + File.open("Gemfile", "w") {|f| f << "source 'file://#{@gem_dir}'\ngem 'tester', :git => '#{@gem_dir}'\n"} bundle_install '--local' + use_config do |config| + config.gem_excludes += [%r{^lib/tester/version}] + end apply_silently expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/lib/tester/version\.rb})).to be_empty expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/tester.gemspec})).to_not be_empty end + it "does not bundle :git entries that are excluded" do + File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rake'\ngroup :test do\ngem 'tester', :git => '#{@gem_dir}'\nend\n"} + bundle_install '--local' + apply_silently + # bundler (>= 2.6) does not need excluded git sources present at all + expect(file_list(%r{WEB-INF/gems/bundler/gems/tester})).to be_empty + end + end context 'with :path entries in the Gemfile' do @@ -107,7 +180,7 @@ def apply_silently expect(file_list(%r{tester})).to be_empty end - it "does work with relative :path" do + it "does work with relative :path, packed once at its relative location" do gem_dir = File.join(Dir.pwd, 'gems/tester') #begin Dir.mkdir(gem_dir) @@ -115,9 +188,13 @@ def apply_silently File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rake'\ngem 'tester', :path => 'gems/tester'\n"} bundle_install '--local' apply_silently - expect(file_list(%r{tester})).to_not be_empty # included from :path as is - expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/lib/tester/version\.rb})).to be_empty - expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/tester.gemspec})).to be_empty + # packed at the Gemfile-relative location bundler resolves at runtime + expect(file_list(%r{WEB-INF/gems/tester/tester\.gemspec})).to_not be_empty + expect(file_list(%r{WEB-INF/gems/tester/lib/tester/version\.rb})).to_not be_empty + # and nowhere else (#465): not in the gem repository nor as a git checkout + expect(file_list(%r{WEB-INF/gems/gems/tester})).to be_empty + expect(file_list(%r{WEB-INF/gems/specifications/tester})).to be_empty + expect(file_list(%r{WEB-INF/gems/bundler/gems/tester})).to be_empty #ensure #FileUtils.rm_r(gem_dir) rescue nil #end @@ -184,11 +261,12 @@ def apply_silently context "when frozen" do run_in_directory "spec/sample_bundler" - it "includes the bundler gem" do + it "does not vendor the (default gem) bundler - the packed JRuby provides it" do bundle_install apply_silently - expect(config.gems.detect{|k,v| k.name == 'bundler'}).to_not be nil - expect(file_list(/bundler-/)).to_not be_empty + expect(config.gems.detect{|k,v| k.name == 'bundler'}).to be nil + expect(file_list(%r{gems/bundler-})).to be_empty + expect(file_list(%r{specifications/bundler-})).to be_empty end it "does not include the bundler cache directory" do @@ -196,10 +274,13 @@ def apply_silently expect(file_list(%r{vendor/bundle})).to be_empty end - it "includes ENV['BUNDLE_FROZEN'] in init.rb" do + it "carries the frozen setting into the packed .bundle/config" do apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings['BUNDLE_FROZEN']).to eq 'true' + # no longer exported via init.rb - the packed .bundle/config is authoritative contents = jar.contents('META-INF/init.rb') - expect(contents.split("\n").grep(/ENV\['BUNDLE_FROZEN'\] = '1'/)).to_not be_empty + expect(contents.split("\n").grep(/BUNDLE_FROZEN/)).to be_empty end context "with the runnable feature" do @@ -249,12 +330,11 @@ def apply_silently bundle 'config', 'set', 'deployment', 'true' end - it "includes the bundler gem" do + it "does not vendor the (default gem) bundler - the packed JRuby provides it" do bundle_install apply_silently expect(file_list(%r{gems/rake-13.4.2/lib})).to_not be_empty - expect(file_list(%r{gems/bundler-})).to_not be_empty - expect(file_list(%r{gems/bundler-.*/exe})).to_not be_empty + expect(file_list(%r{gems/bundler-})).to be_empty end after do @@ -273,3 +353,45 @@ def apply_silently end end end + +describe Warbler::Traits::Bundler, "#warn_on_bundler_version_mismatch" do + require 'tmpdir' + + let(:trait) { Warbler::Traits::Bundler.allocate } + + def lockfile_with_bundled_with(version) + dir = Dir.mktmpdir 'bundled-with' + path = File.join(dir, 'Gemfile.lock') + File.write path, <<-LOCKFILE.gsub(/^ {6}/, '') + GEM + specs: + + PLATFORMS + ruby + + DEPENDENCIES + + BUNDLED WITH + #{version} + LOCKFILE + Pathname.new(path) + end + + it "warns when the lockfile BUNDLED WITH differs from the running bundler" do + allow(::Bundler).to receive(:default_lockfile).and_return lockfile_with_bundled_with('9.9.9') + expect(trait).to receive(:warn).with(/BUNDLED WITH \(9\.9\.9\) does not match the bundler running warbler \(#{Regexp.escape ::Bundler::VERSION}\)/) + trait.send :warn_on_bundler_version_mismatch + end + + it "does not warn when the lockfile BUNDLED WITH matches the running bundler" do + allow(::Bundler).to receive(:default_lockfile).and_return lockfile_with_bundled_with(::Bundler::VERSION) + expect(trait).to_not receive(:warn) + trait.send :warn_on_bundler_version_mismatch + end + + it "does not warn without a lockfile" do + allow(::Bundler).to receive(:default_lockfile).and_return Pathname.new('/no/such/Gemfile.lock') + expect(trait).to_not receive(:warn) + trait.send :warn_on_bundler_version_mismatch + end +end