From 071dd93ef2a5dfd180aad4a0ae8cc628be93582b Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 3 May 2024 13:17:43 +0100 Subject: [PATCH 01/10] env_config: add `HOMEBREW_ALLOWED_TAPS` This is the inverse of `HOMEBREW_FORBIDDEN_TAPS`. --- Library/Homebrew/env_config.rb | 5 ++++ Library/Homebrew/formula_installer.rb | 29 ++++++++++++++----- .../sorbet/rbi/dsl/homebrew/env_config.rbi | 3 ++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/env_config.rb b/Library/Homebrew/env_config.rb index ef9b72ea21..0424212119 100644 --- a/Library/Homebrew/env_config.rb +++ b/Library/Homebrew/env_config.rb @@ -11,6 +11,11 @@ module Homebrew module_function ENVS = { + HOMEBREW_ALLOWED_TAPS: { + description: "A space-separated list of taps. Homebrew will refuse to install a " \ + "formula unless it and all of its dependencies are in an official tap " \ + "or in a tap on this list.", + }, HOMEBREW_API_AUTO_UPDATE_SECS: { description: "Check Homebrew's API for new formulae or cask data every " \ "`HOMEBREW_API_AUTO_UPDATE_SECS` seconds. Alternatively, disable API auto-update " \ diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index e038dd8f50..d73fc59797 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1379,18 +1379,31 @@ on_request: installed_on_request?, options:) EOS end + sig { params(tap: Tap, allowed_taps: T::Set[Tap], forbidden_taps: T::Set[Tap]).returns(T::Boolean) } + def allowed_tap?(tap, allowed_taps, forbidden_taps) + (tap.official? || allowed_taps.blank? || allowed_taps.include?(tap)) && forbidden_taps.exclude?(tap) + end + sig { void } def forbidden_tap_check - forbidden_taps = Homebrew::EnvConfig.forbidden_taps - return if forbidden_taps.blank? + forbidden_taps = Homebrew::EnvConfig.forbidden_taps.to_s.split + allowed_taps = Homebrew::EnvConfig.allowed_taps.to_s.split + return if forbidden_taps.blank? && allowed_taps.blank? - forbidden_taps_set = Set.new(forbidden_taps.split.filter_map do |tap| + forbidden_taps_set = Set.new(forbidden_taps.filter_map do |tap| Tap.fetch(tap) rescue Tap::InvalidNameError opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" nil end) + allowed_taps_set = Set.new(allowed_taps.filter_map do |tap| + Tap.fetch(tap) + rescue Tap::InvalidNameError + opoo "Invalid tap name in `HOMEBREW_ALLOWED_TAPS`: #{tap}" + nil + end) + owner = Homebrew::EnvConfig.forbidden_owner owner_contact = if (contact = Homebrew::EnvConfig.forbidden_owner_contact.presence) "\n#{contact}" @@ -1400,11 +1413,12 @@ on_request: installed_on_request?, options:) compute_dependencies.each do |(dep, _options)| dep_tap = dep.tap next if dep_tap.blank? - next unless forbidden_taps_set.include?(dep_tap) + next if allowed_tap?(dep_tap, allowed_taps_set, forbidden_taps_set) raise CannotInstallFormulaError, <<~EOS The installation of #{formula.name} has a dependency #{dep.name} - but the #{dep_tap} tap was forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} + but #{owner} has either not allowed the #{dep_tap} tap in `HOMEBREW_ALLOWED_TAPS` or + has forbidden the #{dep_tap} tap in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} EOS end end @@ -1413,11 +1427,12 @@ on_request: installed_on_request?, options:) formula_tap = formula.tap return if formula_tap.blank? - return unless forbidden_taps_set.include?(formula_tap) + return if allowed_tap?(formula_tap, allowed_taps_set, forbidden_taps_set) raise CannotInstallFormulaError, <<~EOS The installation of #{formula.full_name} has the tap #{formula_tap} - which was forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} + which is either not allowed by #{owner} in `HOMEBREW_ALLOWED_TAPS` or + is forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} EOS end diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi index 938cd076f0..e6a1945bbe 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi @@ -9,6 +9,9 @@ module Homebrew::EnvConfig sig { returns(T.nilable(::String)) } def all_proxy; end + sig { returns(T.nilable(::String)) } + def allowed_taps; end + sig { returns(Integer) } def api_auto_update_secs; end From 6663516e7948f0f8402147b2c8e7b146500e6d33 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 3 May 2024 14:42:16 +0100 Subject: [PATCH 02/10] tap: define `#allowed_by_env?` --- Library/Homebrew/formula_installer.rb | 29 +++-------------------- Library/Homebrew/tap.rb | 34 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index d73fc59797..f9734eb262 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1379,30 +1379,9 @@ on_request: installed_on_request?, options:) EOS end - sig { params(tap: Tap, allowed_taps: T::Set[Tap], forbidden_taps: T::Set[Tap]).returns(T::Boolean) } - def allowed_tap?(tap, allowed_taps, forbidden_taps) - (tap.official? || allowed_taps.blank? || allowed_taps.include?(tap)) && forbidden_taps.exclude?(tap) - end - sig { void } def forbidden_tap_check - forbidden_taps = Homebrew::EnvConfig.forbidden_taps.to_s.split - allowed_taps = Homebrew::EnvConfig.allowed_taps.to_s.split - return if forbidden_taps.blank? && allowed_taps.blank? - - forbidden_taps_set = Set.new(forbidden_taps.filter_map do |tap| - Tap.fetch(tap) - rescue Tap::InvalidNameError - opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" - nil - end) - - allowed_taps_set = Set.new(allowed_taps.filter_map do |tap| - Tap.fetch(tap) - rescue Tap::InvalidNameError - opoo "Invalid tap name in `HOMEBREW_ALLOWED_TAPS`: #{tap}" - nil - end) + return if Homebrew::EnvConfig.forbidden_taps.blank? && Homebrew::EnvConfig.allowed_taps.blank? owner = Homebrew::EnvConfig.forbidden_owner owner_contact = if (contact = Homebrew::EnvConfig.forbidden_owner_contact.presence) @@ -1412,8 +1391,7 @@ on_request: installed_on_request?, options:) unless ignore_deps? compute_dependencies.each do |(dep, _options)| dep_tap = dep.tap - next if dep_tap.blank? - next if allowed_tap?(dep_tap, allowed_taps_set, forbidden_taps_set) + next if dep_tap.blank? || dep_tap.allowed_by_env? raise CannotInstallFormulaError, <<~EOS The installation of #{formula.name} has a dependency #{dep.name} @@ -1426,8 +1404,7 @@ on_request: installed_on_request?, options:) return if only_deps? formula_tap = formula.tap - return if formula_tap.blank? - return if allowed_tap?(formula_tap, allowed_taps_set, forbidden_taps_set) + return if formula_tap.blank? || formula_tap.allowed_by_env? raise CannotInstallFormulaError, <<~EOS The installation of #{formula.full_name} has the tap #{formula_tap} diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index ab2046dae0..cf3da6e358 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -132,6 +132,30 @@ class Tap false end + sig { returns(T::Set[Tap]) } + def self.allowed_taps + allowed_tap_list = Homebrew::EnvConfig.allowed_taps.to_s.split + + Set.new(allowed_tap_list.filter_map do |tap| + Tap.fetch(tap) + rescue Tap::InvalidNameError + opoo "Invalid tap name in `HOMEBREW_ALLOWED_TAPS`: #{tap}" + nil + end) + end + + sig { returns(T::Set[Tap]) } + def self.forbidden_taps + forbidden_tap_list = Homebrew::EnvConfig.forbidden_taps.to_s.split + + Set.new(forbidden_tap_list.filter_map do |tap| + Tap.fetch(tap) + rescue Tap::InvalidNameError + opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" + nil + end) + end + # @api public extend Enumerable @@ -1056,6 +1080,16 @@ class Tap end end + sig { returns(T::Boolean) } + def allowed_by_env? + @allowed_by_env ||= begin + allowed_taps = self.class.allowed_taps + forbidden_taps = self.class.forbidden_taps + + (official? || allowed_taps.blank? || allowed_taps.include?(self)) && forbidden_taps.exclude?(self) + end + end + private sig { params(file: Pathname).returns(T.any(T::Array[String], Hash)) } From 34e2c4ee97ba3484df7814ba323a9b767cb997f8 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 3 May 2024 14:48:17 +0100 Subject: [PATCH 03/10] cask/installer: support `HOMEBREW_ALLOWED_TAPS` --- Library/Homebrew/cask/installer.rb | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index b7d4309ca9..0e2e89396c 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -574,15 +574,7 @@ on_request: true) sig { void } def forbidden_tap_check - forbidden_taps = Homebrew::EnvConfig.forbidden_taps - return if forbidden_taps.blank? - - forbidden_taps_set = Set.new(forbidden_taps.split.filter_map do |tap| - Tap.fetch(tap) - rescue Tap::InvalidNameError - opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" - nil - end) + return if Homebrew::EnvConfig.forbidden_taps.blank? && Homebrew::EnvConfig.allowed_taps.blank? owner = Homebrew::EnvConfig.forbidden_owner owner_contact = if (contact = Homebrew::EnvConfig.forbidden_owner_contact.presence) @@ -592,25 +584,25 @@ on_request: true) unless skip_cask_deps? cask_and_formula_dependencies.each do |cask_or_formula| dep_tap = cask_or_formula.tap - next if dep_tap.blank? - next unless forbidden_taps_set.include?(dep_tap) + next if dep_tap.blank? || dep_tap.allowed_by_env? dep_full_name = cask_or_formula.full_name raise CaskCannotBeInstalledError.new(@cask, <<~EOS The installation of #{@cask} has a dependency #{dep_full_name} - but the #{dep_tap} tap was forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} + but #{owner} has either not allowed the #{dep_tap} in `HOMEBREW_ALLOWED_TAPS` or + has forbidden the #{dep_tap} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} EOS ) end end cask_tap = @cask.tap - return if cask_tap.blank? - return unless forbidden_taps_set.include?(cask_tap) + return if cask_tap.blank? || cask_tap.allowed_by_env? raise CaskCannotBeInstalledError.new(@cask, <<~EOS The installation of #{@cask.full_name} has the tap #{cask_tap} - which was forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} + which is either not allowed by #{owner} in `HOMEBREW_ALLOWED_TAPS` or + is forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} EOS ) end From 7c9e8927e94e8637eb7c75ce87ca78ffe204f73c Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 3 May 2024 16:08:22 +0100 Subject: [PATCH 04/10] tap: memoize allowed and forbidden taps --- Library/Homebrew/tap.rb | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index cf3da6e358..527fc0e156 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -132,29 +132,40 @@ class Tap false end + # rubocop:disable Style/ClassVars + # We want the the class variables below to be global to all `Tap` classes and subclasses. sig { returns(T::Set[Tap]) } def self.allowed_taps - allowed_tap_list = Homebrew::EnvConfig.allowed_taps.to_s.split + @@allowed_taps ||= begin + allowed_tap_list = Homebrew::EnvConfig.allowed_taps.to_s.split - Set.new(allowed_tap_list.filter_map do |tap| - Tap.fetch(tap) - rescue Tap::InvalidNameError - opoo "Invalid tap name in `HOMEBREW_ALLOWED_TAPS`: #{tap}" - nil - end) + Set.new(allowed_tap_list.filter_map do |tap| + Tap.fetch(tap) + rescue Tap::InvalidNameError + opoo "Invalid tap name in `HOMEBREW_ALLOWED_TAPS`: #{tap}" + nil + end) + end + + @@allowed_taps.freeze end sig { returns(T::Set[Tap]) } def self.forbidden_taps - forbidden_tap_list = Homebrew::EnvConfig.forbidden_taps.to_s.split + @@forbidden_taps ||= begin + forbidden_tap_list = Homebrew::EnvConfig.forbidden_taps.to_s.split - Set.new(forbidden_tap_list.filter_map do |tap| - Tap.fetch(tap) - rescue Tap::InvalidNameError - opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" - nil - end) + Set.new(forbidden_tap_list.filter_map do |tap| + Tap.fetch(tap) + rescue Tap::InvalidNameError + opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" + nil + end) + end + + @@forbidden_taps.freeze end + # rubocop:enable Style/ClassVars # @api public extend Enumerable From 5222c9e32d1ab9eeb05a4ab3ef23c958ce56044a Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 3 May 2024 16:08:42 +0100 Subject: [PATCH 05/10] Improve error message for allowed and forbidden taps --- Library/Homebrew/cask/installer.rb | 30 ++++++++++++++----------- Library/Homebrew/formula_installer.rb | 32 ++++++++++++++++----------- Library/Homebrew/tap.rb | 8 +++++-- 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 0e2e89396c..9b00efab5e 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -574,7 +574,7 @@ on_request: true) sig { void } def forbidden_tap_check - return if Homebrew::EnvConfig.forbidden_taps.blank? && Homebrew::EnvConfig.allowed_taps.blank? + return if Tap.allowed_taps.blank? && Tap.forbidden_taps.blank? owner = Homebrew::EnvConfig.forbidden_owner owner_contact = if (contact = Homebrew::EnvConfig.forbidden_owner_contact.presence) @@ -587,24 +587,28 @@ on_request: true) next if dep_tap.blank? || dep_tap.allowed_by_env? dep_full_name = cask_or_formula.full_name - raise CaskCannotBeInstalledError.new(@cask, <<~EOS - The installation of #{@cask} has a dependency #{dep_full_name} - but #{owner} has either not allowed the #{dep_tap} in `HOMEBREW_ALLOWED_TAPS` or - has forbidden the #{dep_tap} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} - EOS - ) + error_message = +"The installation of #{@cask} has a dependency #{dep_full_name}\n" \ + "from the #{dep_tap} tap but #{owner} " + error_message << "has not allowed this tap in `HOMEBREW_ALLOWED_TAPS`" unless dep_tap.allowed_by_env? + error_message << " and\n" if !dep_tap.allowed_by_env? && dep_tap.forbidden_by_env? + error_message << "has forbidden this tap in `HOMEBREW_FORBIDDEN_TAPS`" if dep_tap.forbidden_by_env? + error_message << ".#{owner_contact}" + + raise CaskCannotBeInstalledError.new(@cask, error_message) end end cask_tap = @cask.tap return if cask_tap.blank? || cask_tap.allowed_by_env? - raise CaskCannotBeInstalledError.new(@cask, <<~EOS - The installation of #{@cask.full_name} has the tap #{cask_tap} - which is either not allowed by #{owner} in `HOMEBREW_ALLOWED_TAPS` or - is forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} - EOS - ) + error_message = +"The installation of #{@cask.full_name} has the tap #{cask_tap}\n" \ + "but #{owner} " + error_message << "has not allowed this tap in `HOMEBREW_ALLOWED_TAPS`" unless cask_tap.allowed_by_env? + error_message << " and\n" if !cask_tap.allowed_by_env? && cask_tap.forbidden_by_env? + error_message << "has forbidden this tap in `HOMEBREW_FORBIDDEN_TAPS`" if cask_tap.forbidden_by_env? + error_message << ".#{owner_contact}" + + raise CaskCannotBeInstalledError.new(@cask, error_message) end sig { void } diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index f9734eb262..b417b5d941 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1381,7 +1381,7 @@ on_request: installed_on_request?, options:) sig { void } def forbidden_tap_check - return if Homebrew::EnvConfig.forbidden_taps.blank? && Homebrew::EnvConfig.allowed_taps.blank? + return if Tap.allowed_taps.blank? && Tap.forbidden_taps.blank? owner = Homebrew::EnvConfig.forbidden_owner owner_contact = if (contact = Homebrew::EnvConfig.forbidden_owner_contact.presence) @@ -1391,26 +1391,32 @@ on_request: installed_on_request?, options:) unless ignore_deps? compute_dependencies.each do |(dep, _options)| dep_tap = dep.tap - next if dep_tap.blank? || dep_tap.allowed_by_env? + next if dep_tap.blank? || (dep_tap.allowed_by_env? && !dep_tap.forbidden_by_env?) - raise CannotInstallFormulaError, <<~EOS - The installation of #{formula.name} has a dependency #{dep.name} - but #{owner} has either not allowed the #{dep_tap} tap in `HOMEBREW_ALLOWED_TAPS` or - has forbidden the #{dep_tap} tap in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} - EOS + error_message = +"The installation of #{formula.name} has a dependency #{dep.name}\n" \ + "from the #{dep_tap} tap but #{owner} " + error_message << "has not allowed this tap in `HOMEBREW_ALLOWED_TAPS`" unless dep_tap.allowed_by_env? + error_message << " and\n" if !dep_tap.allowed_by_env? && dep_tap.forbidden_by_env? + error_message << "has forbidden this tap in `HOMEBREW_FORBIDDEN_TAPS`" if dep_tap.forbidden_by_env? + error_message << ".#{owner_contact}" + + raise CannotInstallFormulaError, error_message end end return if only_deps? formula_tap = formula.tap - return if formula_tap.blank? || formula_tap.allowed_by_env? + return if formula_tap.blank? || (formula_tap.allowed_by_env? && !formula_tap.forbidden_by_env?) - raise CannotInstallFormulaError, <<~EOS - The installation of #{formula.full_name} has the tap #{formula_tap} - which is either not allowed by #{owner} in `HOMEBREW_ALLOWED_TAPS` or - is forbidden by #{owner} in `HOMEBREW_FORBIDDEN_TAPS`.#{owner_contact} - EOS + error_message = +"The installation of #{formula.full_name} has the tap #{formula_tap}\n" \ + "but #{owner} " + error_message << "has not allowed this tap in `HOMEBREW_ALLOWED_TAPS`" unless formula_tap.allowed_by_env? + error_message << " and\n" if !formula_tap.allowed_by_env? && formula_tap.forbidden_by_env? + error_message << "has forbidden this tap in `HOMEBREW_FORBIDDEN_TAPS`" if formula_tap.forbidden_by_env? + error_message << ".#{owner_contact}" + + raise CannotInstallFormulaError, error_message end sig { void } diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index 527fc0e156..b386eaa985 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -1095,12 +1095,16 @@ class Tap def allowed_by_env? @allowed_by_env ||= begin allowed_taps = self.class.allowed_taps - forbidden_taps = self.class.forbidden_taps - (official? || allowed_taps.blank? || allowed_taps.include?(self)) && forbidden_taps.exclude?(self) + official? || allowed_taps.blank? || allowed_taps.include?(self) end end + sig { returns(T::Boolean) } + def forbidden_by_env? + @forbidden_by_env ||= self.class.forbidden_taps.include?(self) + end + private sig { params(file: Pathname).returns(T.any(T::Array[String], Hash)) } From 3555d09c1d4048caa27f585c979bed36b4ceda31 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 3 May 2024 16:43:39 +0100 Subject: [PATCH 06/10] formula_installer: fix failing test --- Library/Homebrew/test/formula_installer_spec.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb index e8be7f7cbb..611a5546b5 100644 --- a/Library/Homebrew/test/formula_installer_spec.rb +++ b/Library/Homebrew/test/formula_installer_spec.rb @@ -258,10 +258,17 @@ RSpec.describe FormulaInstaller do end describe "#forbidden_tap_check" do + before do + allow(Tap).to receive(:forbidden_taps).and_return(forbidden_taps_set) + end + + let(:homebrew_forbidden) { Tap.fetch("homebrew/forbidden") } + let(:forbidden_taps_set) { Set.new([homebrew_forbidden]) } + it "raises on forbidden tap on formula" do - ENV["HOMEBREW_FORBIDDEN_TAPS"] = f_tap = "homebrew/forbidden" + f_tap = homebrew_forbidden f_name = "homebrew-forbidden-tap" - f_path = Tap.fetch(f_tap).new_formula_path(f_name) + f_path = homebrew_forbidden.new_formula_path(f_name) f_path.parent.mkpath f_path.write <<~RUBY class #{Formulary.class_s(f_name)} < Formula @@ -282,9 +289,9 @@ RSpec.describe FormulaInstaller do end it "raises on forbidden tap on dependency" do - ENV["HOMEBREW_FORBIDDEN_TAPS"] = dep_tap = "homebrew/forbidden" + dep_tap = homebrew_forbidden dep_name = "homebrew-forbidden-dependency-tap" - dep_path = Tap.fetch(dep_tap).new_formula_path(dep_name) + dep_path = homebrew_forbidden.new_formula_path(dep_name) dep_path.parent.mkpath dep_path.write <<~RUBY class #{Formulary.class_s(dep_name)} < Formula @@ -310,7 +317,7 @@ RSpec.describe FormulaInstaller do expect do fi.forbidden_tap_check - end.to raise_error(CannotInstallFormulaError, /but the #{dep_tap} tap was forbidden/) + end.to raise_error(CannotInstallFormulaError, /from the #{dep_tap} tap but/) ensure dep_path.parent.parent.rmtree end From 34387bfc8a355db698d181aff68bc779ae239d0d Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Sun, 5 May 2024 13:54:26 +0100 Subject: [PATCH 07/10] cask/installer: update to match formula_installer --- Library/Homebrew/cask/installer.rb | 4 ++-- Library/Homebrew/test/cask/installer_spec.rb | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 9b00efab5e..123796b456 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -584,7 +584,7 @@ on_request: true) unless skip_cask_deps? cask_and_formula_dependencies.each do |cask_or_formula| dep_tap = cask_or_formula.tap - next if dep_tap.blank? || dep_tap.allowed_by_env? + next if dep_tap.blank? || (dep_tap.allowed_by_env? && !dep_tap.forbidden_by_env?) dep_full_name = cask_or_formula.full_name error_message = +"The installation of #{@cask} has a dependency #{dep_full_name}\n" \ @@ -599,7 +599,7 @@ on_request: true) end cask_tap = @cask.tap - return if cask_tap.blank? || cask_tap.allowed_by_env? + return if cask_tap.blank? || (cask_tap.allowed_by_env? && !cask_tap.forbidden_by_env?) error_message = +"The installation of #{@cask.full_name} has the tap #{cask_tap}\n" \ "but #{owner} " diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index 031b9d4a99..b453da9246 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -326,22 +326,27 @@ RSpec.describe Cask::Installer, :cask do end describe "#forbidden_tap_check" do - it "raises on forbidden tap on cask" do - ENV["HOMEBREW_FORBIDDEN_TAPS"] = tap = "homebrew/forbidden" + before do + allow(Tap).to receive(:forbidden_taps).and_return(forbidden_taps_set) + end - cask = Cask::Cask.new("homebrew-forbidden-tap", tap: Tap.fetch(tap)) do + let(:homebrew_forbidden) { Tap.fetch("homebrew/forbidden") } + let(:forbidden_taps_set) { Set.new([homebrew_forbidden]) } + + it "raises on forbidden tap on cask" do + cask = Cask::Cask.new("homebrew-forbidden-tap", tap: homebrew_forbidden) do url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz" end expect do described_class.new(cask).forbidden_tap_check - end.to raise_error(Cask::CaskCannotBeInstalledError, /has the tap #{tap}/) + end.to raise_error(Cask::CaskCannotBeInstalledError, /has the tap #{homebrew_forbidden}/) end it "raises on forbidden tap on dependency" do - ENV["HOMEBREW_FORBIDDEN_TAPS"] = dep_tap = "homebrew/forbidden" + dep_tap = homebrew_forbidden dep_name = "homebrew-forbidden-dependency-tap" - dep_path = Tap.fetch(dep_tap).new_formula_path(dep_name) + dep_path = dep_tap.new_formula_path(dep_name) dep_path.parent.mkpath dep_path.write <<~RUBY class #{Formulary.class_s(dep_name)} < Formula @@ -358,7 +363,7 @@ RSpec.describe Cask::Installer, :cask do expect do described_class.new(cask).forbidden_tap_check - end.to raise_error(Cask::CaskCannotBeInstalledError, /but the #{dep_tap} tap was forbidden/) + end.to raise_error(Cask::CaskCannotBeInstalledError, /from the #{dep_tap} tap but/) ensure dep_path.parent.parent.rmtree end From 078a328e8e221828080db50abcdbb3e94cb3ea07 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Mon, 6 May 2024 15:05:06 +0100 Subject: [PATCH 08/10] tap: avoid class vars Avoiding them also allows us to write proper tests. --- Library/Homebrew/tap.rb | 17 ++++++----------- Library/Homebrew/test/tap_spec.rb | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index b386eaa985..f66261d14b 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -132,11 +132,10 @@ class Tap false end - # rubocop:disable Style/ClassVars - # We want the the class variables below to be global to all `Tap` classes and subclasses. sig { returns(T::Set[Tap]) } def self.allowed_taps - @@allowed_taps ||= begin + cache_key = "allowed_taps_#{Homebrew::EnvConfig.allowed_taps.to_s.gsub(" ", "_")}".to_sym + cache[cache_key] ||= begin allowed_tap_list = Homebrew::EnvConfig.allowed_taps.to_s.split Set.new(allowed_tap_list.filter_map do |tap| @@ -144,15 +143,14 @@ class Tap rescue Tap::InvalidNameError opoo "Invalid tap name in `HOMEBREW_ALLOWED_TAPS`: #{tap}" nil - end) + end).freeze end - - @@allowed_taps.freeze end sig { returns(T::Set[Tap]) } def self.forbidden_taps - @@forbidden_taps ||= begin + cache_key = "forbidden_taps_#{Homebrew::EnvConfig.forbidden_taps.to_s.gsub(" ", "_")}".to_sym + cache[cache_key] ||= begin forbidden_tap_list = Homebrew::EnvConfig.forbidden_taps.to_s.split Set.new(forbidden_tap_list.filter_map do |tap| @@ -160,12 +158,9 @@ class Tap rescue Tap::InvalidNameError opoo "Invalid tap name in `HOMEBREW_FORBIDDEN_TAPS`: #{tap}" nil - end) + end).freeze end - - @@forbidden_taps.freeze end - # rubocop:enable Style/ClassVars # @api public extend Enumerable diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index a8cafb6799..940ac944f9 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -142,6 +142,24 @@ RSpec.describe Tap do end end + describe "::allowed_taps" do + before { allow(Homebrew::EnvConfig).to receive(:allowed_taps).and_return("homebrew/allowed") } + + it "returns a set of allowed taps according to the environment" do + expect(described_class.allowed_taps) + .to contain_exactly(described_class.fetch("homebrew/allowed")) + end + end + + describe "::forbidden_taps" do + before { allow(Homebrew::EnvConfig).to receive(:forbidden_taps).and_return("homebrew/forbidden") } + + it "returns a set of forbidden taps according to the environment" do + expect(described_class.forbidden_taps) + .to contain_exactly(described_class.fetch("homebrew/forbidden")) + end + end + specify "::names" do expect(described_class.names.sort).to eq(["homebrew/core", "homebrew/foo"]) end From 3b794fc6e895bca8997d61ccd0a1fd2a285a75a2 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Mon, 6 May 2024 15:22:48 +0100 Subject: [PATCH 09/10] formula_installer, cask/installer: add tests for `HOMEBREW_ALLOWED_TAPS` --- Library/Homebrew/test/cask/installer_spec.rb | 22 +++++++++ .../Homebrew/test/formula_installer_spec.rb | 48 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index b453da9246..b0e22b9b92 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -327,10 +327,14 @@ RSpec.describe Cask::Installer, :cask do describe "#forbidden_tap_check" do before do + allow(Tap).to receive(:allowed_taps).and_return(allowed_taps_set) allow(Tap).to receive(:forbidden_taps).and_return(forbidden_taps_set) end let(:homebrew_forbidden) { Tap.fetch("homebrew/forbidden") } + let(:allowed_third_party) { Tap.fetch("nothomebrew/allowed") } + let(:disallowed_third_party) { Tap.fetch("nothomebrew/notallowed") } + let(:allowed_taps_set) { Set.new([allowed_third_party]) } let(:forbidden_taps_set) { Set.new([homebrew_forbidden]) } it "raises on forbidden tap on cask" do @@ -343,6 +347,24 @@ RSpec.describe Cask::Installer, :cask do end.to raise_error(Cask::CaskCannotBeInstalledError, /has the tap #{homebrew_forbidden}/) end + it "raises on not allowed third-party tap on cask" do + cask = Cask::Cask.new("homebrew-not-allowed-tap", tap: disallowed_third_party) do + url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz" + end + + expect do + described_class.new(cask).forbidden_tap_check + end.to raise_error(Cask::CaskCannotBeInstalledError, /has the tap #{disallowed_third_party}/) + end + + it "does not raise on allowed tap on cask" do + cask = Cask::Cask.new("third-party-allowed-tap", tap: allowed_third_party) do + url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz" + end + + expect { described_class.new(cask).forbidden_tap_check }.not_to raise_error + end + it "raises on forbidden tap on dependency" do dep_tap = homebrew_forbidden dep_name = "homebrew-forbidden-dependency-tap" diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb index 611a5546b5..d0878a3e3c 100644 --- a/Library/Homebrew/test/formula_installer_spec.rb +++ b/Library/Homebrew/test/formula_installer_spec.rb @@ -259,10 +259,14 @@ RSpec.describe FormulaInstaller do describe "#forbidden_tap_check" do before do + allow(Tap).to receive(:allowed_taps).and_return(allowed_taps_set) allow(Tap).to receive(:forbidden_taps).and_return(forbidden_taps_set) end let(:homebrew_forbidden) { Tap.fetch("homebrew/forbidden") } + let(:allowed_third_party) { Tap.fetch("nothomebrew/allowed") } + let(:disallowed_third_party) { Tap.fetch("nothomebrew/notallowed") } + let(:allowed_taps_set) { Set.new([allowed_third_party]) } let(:forbidden_taps_set) { Set.new([homebrew_forbidden]) } it "raises on forbidden tap on formula" do @@ -288,6 +292,50 @@ RSpec.describe FormulaInstaller do f_path.parent.parent.rmtree end + it "raises on not allowed third-party tap on formula" do + f_tap = disallowed_third_party + f_name = "homebrew-not-allowed-tap" + f_path = disallowed_third_party.new_formula_path(f_name) + f_path.parent.mkpath + f_path.write <<~RUBY + class #{Formulary.class_s(f_name)} < Formula + url "foo" + version "0.1" + end + RUBY + Formulary.cache.delete(f_path) + + f = Formulary.factory("#{f_tap}/#{f_name}") + fi = described_class.new(f) + + expect do + fi.forbidden_tap_check + end.to raise_error(CannotInstallFormulaError, /has the tap #{f_tap}/) + ensure + f_path.parent.parent.parent.rmtree + end + + it "does not raise on allowed tap on formula" do + f_tap = allowed_third_party + f_name = "homebrew-allowed-tap" + f_path = allowed_third_party.new_formula_path(f_name) + f_path.parent.mkpath + f_path.write <<~RUBY + class #{Formulary.class_s(f_name)} < Formula + url "foo" + version "0.1" + end + RUBY + Formulary.cache.delete(f_path) + + f = Formulary.factory("#{f_tap}/#{f_name}") + fi = described_class.new(f) + + expect { fi.forbidden_tap_check }.not_to raise_error + ensure + f_path.parent.parent.parent.rmtree + end + it "raises on forbidden tap on dependency" do dep_tap = homebrew_forbidden dep_name = "homebrew-forbidden-dependency-tap" From be29afa5f7f3845c8af836598ca6d2f3a3db8fb7 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Mon, 6 May 2024 15:26:29 +0100 Subject: [PATCH 10/10] Fix `brew style` --- Library/Homebrew/tap.rb | 4 ++-- Library/Homebrew/test/cask/installer_spec.rb | 3 +-- Library/Homebrew/test/formula_installer_spec.rb | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index f66261d14b..ec38eb9292 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -134,7 +134,7 @@ class Tap sig { returns(T::Set[Tap]) } def self.allowed_taps - cache_key = "allowed_taps_#{Homebrew::EnvConfig.allowed_taps.to_s.gsub(" ", "_")}".to_sym + cache_key = :"allowed_taps_#{Homebrew::EnvConfig.allowed_taps.to_s.tr(" ", "_")}" cache[cache_key] ||= begin allowed_tap_list = Homebrew::EnvConfig.allowed_taps.to_s.split @@ -149,7 +149,7 @@ class Tap sig { returns(T::Set[Tap]) } def self.forbidden_taps - cache_key = "forbidden_taps_#{Homebrew::EnvConfig.forbidden_taps.to_s.gsub(" ", "_")}".to_sym + cache_key = :"forbidden_taps_#{Homebrew::EnvConfig.forbidden_taps.to_s.tr(" ", "_")}" cache[cache_key] ||= begin forbidden_tap_list = Homebrew::EnvConfig.forbidden_taps.to_s.split diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index b0e22b9b92..568d848909 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -327,8 +327,7 @@ RSpec.describe Cask::Installer, :cask do describe "#forbidden_tap_check" do before do - allow(Tap).to receive(:allowed_taps).and_return(allowed_taps_set) - allow(Tap).to receive(:forbidden_taps).and_return(forbidden_taps_set) + allow(Tap).to receive_messages(allowed_taps: allowed_taps_set, forbidden_taps: forbidden_taps_set) end let(:homebrew_forbidden) { Tap.fetch("homebrew/forbidden") } diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb index d0878a3e3c..e2655cd0c5 100644 --- a/Library/Homebrew/test/formula_installer_spec.rb +++ b/Library/Homebrew/test/formula_installer_spec.rb @@ -259,8 +259,7 @@ RSpec.describe FormulaInstaller do describe "#forbidden_tap_check" do before do - allow(Tap).to receive(:allowed_taps).and_return(allowed_taps_set) - allow(Tap).to receive(:forbidden_taps).and_return(forbidden_taps_set) + allow(Tap).to receive_messages(allowed_taps: allowed_taps_set, forbidden_taps: forbidden_taps_set) end let(:homebrew_forbidden) { Tap.fetch("homebrew/forbidden") }