From 32b46293010adfc904da6eb21c0cd5c973164f99 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 8 Jun 2026 21:02:25 +0100 Subject: [PATCH 1/3] Make `brew bundle` honour the `trusted:` option - `trusted: true` was parsed but ignored on install, so a formula or cask from an untrusted tap still failed to load mid-install - persist trust for the formula or cask before it is loaded so the in-process conflict check and the `brew install` subprocess see it trusted - crucially, trust the formula before `tap.ensure_installed!`: tapping loads the formula and triggers the trust check, so trusting only afterwards left the very first `brew bundle` run failing - this completes the round-trip with `brew bundle dump`, which already emits `trusted: true` for trusted entries --- Library/Homebrew/bundle/brew.rb | 8 +++++++ Library/Homebrew/bundle/cask.rb | 6 ++++- Library/Homebrew/test/bundle/brew_spec.rb | 29 +++++++++++++++++++++++ Library/Homebrew/test/bundle/cask_spec.rb | 12 ++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/bundle/brew.rb b/Library/Homebrew/bundle/brew.rb index df37b10e71c78..0836184c8c2d0 100644 --- a/Library/Homebrew/bundle/brew.rb +++ b/Library/Homebrew/bundle/brew.rb @@ -456,6 +456,7 @@ def initialize(name = "", options = {}) @link = T.let(options.fetch(:link, nil), T.nilable(T.any(Symbol, T::Boolean))) @postinstall = T.let(options.fetch(:postinstall, nil), T.nilable(String)) @version_file = T.let(options.fetch(:version_file, nil), T.nilable(String)) + @trusted = T.let(options.fetch(:trusted, false), T::Boolean) @changed = T.let(nil, T.nilable(T::Boolean)) end @@ -552,6 +553,13 @@ def install!(preinstall: true, no_upgrade: false, verbose: false, force: false) sig { params(no_upgrade: T::Boolean, verbose: T::Boolean, force: T::Boolean).returns(T::Boolean) } def install_change_state!(no_upgrade:, verbose:, force:) require "tap" + + # Trust before tapping: installing the tap loads the formula, which + # triggers the trust check before any later step could grant trust. + # Only fully-qualified names map to a tap, so unqualified names cannot + # be meaningfully trusted. + Homebrew::Trust.trust!(:formula, @full_name) if @trusted && @full_name.count("/") == 2 + if (tap_with_name = ::Tap.with_formula_name(@full_name)) tap, = tap_with_name tap.ensure_installed! diff --git a/Library/Homebrew/bundle/cask.rb b/Library/Homebrew/bundle/cask.rb index 52d5cf572a31e..ccb66c6616260 100644 --- a/Library/Homebrew/bundle/cask.rb +++ b/Library/Homebrew/bundle/cask.rb @@ -95,7 +95,11 @@ def preinstall!(name, no_upgrade: false, verbose: false, **options) def install!(name, preinstall: true, no_upgrade: false, verbose: false, force: false, **options) return true unless preinstall - full_name = options.fetch(:full_name, name) + full_name = T.cast(options.fetch(:full_name, name), String) + + # Only fully-qualified names map to a tap, so unqualified tokens + # cannot be meaningfully trusted. + Homebrew::Trust.trust!(:cask, full_name) if options[:trusted] && full_name.count("/") == 2 install_result = if cask_installed?(name) && upgrading?(no_upgrade, name, options) status = "#{options[:greedy] ? "may not be" : "not"} up-to-date" diff --git a/Library/Homebrew/test/bundle/brew_spec.rb b/Library/Homebrew/test/bundle/brew_spec.rb index c0c889ca9d6dc..2c3c57515c466 100644 --- a/Library/Homebrew/test/bundle/brew_spec.rb +++ b/Library/Homebrew/test/bundle/brew_spec.rb @@ -278,6 +278,10 @@ let(:installer) { described_class.new(formula_name, options) } before do + # Clear the class-level formula cache so a hash memoised by an earlier + # example (e.g. without conflicts) doesn't leak into this one. + described_class.reset! + # don't try to load gcc/glibc allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false) @@ -578,6 +582,31 @@ end end + context "when the trusted option is true" do + let(:tapped_name) { "foo/bar/baz" } + + before do + allow_any_instance_of(described_class).to receive_messages(installed?: false, resolve_conflicts!: true, + install_formula!: true) + end + + it "trusts the formula before installing the tap that loads it" do + order = [] + tap = instance_double(Tap, ensure_installed!: nil) + allow(Tap).to receive(:with_formula_name).with(tapped_name).and_return([tap, "baz"]) + allow(tap).to receive(:ensure_installed!) { order << :tap } + allow(Homebrew::Trust).to receive(:trust!).with(:formula, tapped_name) { order << :trust } + described_class.install!(tapped_name, trusted: true) + expect(order).to eq([:trust, :tap]) + end + + it "does not trust an unqualified formula name" do + allow(Tap).to receive(:with_formula_name).and_return(nil) + expect(Homebrew::Trust).not_to receive(:trust!) + described_class.install!("baz", trusted: true) + end + end + describe ".outdated_formulae" do it "calls Homebrew" do described_class.reset! diff --git a/Library/Homebrew/test/bundle/cask_spec.rb b/Library/Homebrew/test/bundle/cask_spec.rb index 0af7f4e3a928d..5b34e66ac9d89 100644 --- a/Library/Homebrew/test/bundle/cask_spec.rb +++ b/Library/Homebrew/test/bundle/cask_spec.rb @@ -258,6 +258,18 @@ expect(described_class.install!("google-chrome")).to be(true) end + it "trusts the cask before installing it" do + allow(Homebrew::Bundle).to receive(:brew).and_return(true) + expect(Homebrew::Trust).to receive(:trust!).with(:cask, "puma/puma/puma-cask") + described_class.install!("puma-cask", full_name: "puma/puma/puma-cask", trusted: true) + end + + it "does not trust an unqualified cask token" do + allow(Homebrew::Bundle).to receive(:brew).and_return(true) + expect(Homebrew::Trust).not_to receive(:trust!) + described_class.install!("google-chrome", trusted: true) + end + it "installs cask with arguments" do expect(Homebrew::Bundle).to( receive(:brew).with("install", "--cask", "firefox", "--appdir=/Applications", "--adopt", From 3446e7ba464141cf2417a670882650c5843bcd6f Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 8 Jun 2026 21:02:26 +0100 Subject: [PATCH 2/3] Add `Utils.full_name?` tap-name helper - the fully-qualified name check `count("/") == 2` was duplicated across `trust.rb`, `tap.rb` and `brew bundle` - extract it next to `name_from_full_name`/`tap_from_full_name` and reuse it so the intent reads clearly at each call site --- Library/Homebrew/bundle/brew.rb | 6 +++--- Library/Homebrew/bundle/cask.rb | 2 +- Library/Homebrew/tap.rb | 2 +- Library/Homebrew/test/utils_spec.rb | 14 ++++++++++++++ Library/Homebrew/trust.rb | 2 +- Library/Homebrew/utils.rb | 6 ++++++ 6 files changed, 26 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/bundle/brew.rb b/Library/Homebrew/bundle/brew.rb index 0836184c8c2d0..87dcb8538870f 100644 --- a/Library/Homebrew/bundle/brew.rb +++ b/Library/Homebrew/bundle/brew.rb @@ -134,7 +134,7 @@ def formula_in_array?(formula, array) def formula_installed?(formula) # Fully qualified tap formulae can be checked by their Cellar rack name # without loading the formula from an untrusted tap. - return installed_formulae.include?(Utils.name_from_full_name(formula)) if formula.count("/") == 2 + return installed_formulae.include?(Utils.name_from_full_name(formula)) if Utils.full_name?(formula) formula_in_array?(formula, installed_formulae) end @@ -145,7 +145,7 @@ def formula_upgradable?(formula) # Reading the formula is needed for authoritative outdated state, so # report trust problems before the upgrade check tries to load it. - if formula.count("/") == 2 && Homebrew::EnvConfig.require_tap_trust? + if Utils.full_name?(formula) && Homebrew::EnvConfig.require_tap_trust? require "trust" unless Homebrew::Trust.trusted?(:formula, formula) @@ -558,7 +558,7 @@ def install_change_state!(no_upgrade:, verbose:, force:) # triggers the trust check before any later step could grant trust. # Only fully-qualified names map to a tap, so unqualified names cannot # be meaningfully trusted. - Homebrew::Trust.trust!(:formula, @full_name) if @trusted && @full_name.count("/") == 2 + Homebrew::Trust.trust!(:formula, @full_name) if @trusted && Utils.full_name?(@full_name) if (tap_with_name = ::Tap.with_formula_name(@full_name)) tap, = tap_with_name diff --git a/Library/Homebrew/bundle/cask.rb b/Library/Homebrew/bundle/cask.rb index ccb66c6616260..4d73c1d709eab 100644 --- a/Library/Homebrew/bundle/cask.rb +++ b/Library/Homebrew/bundle/cask.rb @@ -99,7 +99,7 @@ def install!(name, preinstall: true, no_upgrade: false, verbose: false, force: f # Only fully-qualified names map to a tap, so unqualified tokens # cannot be meaningfully trusted. - Homebrew::Trust.trust!(:cask, full_name) if options[:trusted] && full_name.count("/") == 2 + Homebrew::Trust.trust!(:cask, full_name) if options[:trusted] && Utils.full_name?(full_name) install_result = if cask_installed?(name) && upgrading?(no_upgrade, name, options) status = "#{options[:greedy] ? "may not be" : "not"} up-to-date" diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index bd410f7a6950b..5138af10e4eb0 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -1095,7 +1095,7 @@ def reverse_tap_migrations_renames # Only include renames: # + `homebrew/cask/water-buffalo` # - `homebrew/cask` - next if new_name.count("/") != 2 + next unless Utils.full_name?(new_name) hash[new_name] ||= [] hash[new_name] << old_name diff --git a/Library/Homebrew/test/utils_spec.rb b/Library/Homebrew/test/utils_spec.rb index 581be040f4297..d7e6918d669f4 100644 --- a/Library/Homebrew/test/utils_spec.rb +++ b/Library/Homebrew/test/utils_spec.rb @@ -65,6 +65,20 @@ end end + describe ".full_name?" do + it "is true for a fully-qualified name" do + expect(described_class.full_name?("homebrew/core/wget")).to be(true) + end + + it "is false for an unqualified name" do + expect(described_class.full_name?("wget")).to be(false) + end + + it "is false for a tap name" do + expect(described_class.full_name?("homebrew/core")).to be(false) + end + end + specify ".parse_author!" do parse_error_msg = /Unable to parse name and email/ diff --git a/Library/Homebrew/trust.rb b/Library/Homebrew/trust.rb index 761a679f1c755..2767a1184a2d8 100644 --- a/Library/Homebrew/trust.rb +++ b/Library/Homebrew/trust.rb @@ -59,7 +59,7 @@ def self.untrust!(type, name) sig { params(names: T::Array[String], type: T.nilable(Symbol)).void } def self.trust_fully_qualified_items!(names, type: nil) names.each do |name| - next if name.count("/") != 2 + next unless ::Utils.full_name?(name) tap_name = name.split("/").first(2).join("/") item_name = ::Utils.name_from_full_name(name) diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb index 71ccfa0faa603..424d960c02b4e 100644 --- a/Library/Homebrew/utils.rb +++ b/Library/Homebrew/utils.rb @@ -60,6 +60,12 @@ def self.tap_from_full_name(full_name) "#{user}/#{repository}" end + # Whether `full_name` is fully-qualified with a tap prefix, e.g. `user/tap/name`. + sig { params(full_name: String).returns(T::Boolean) } + def self.full_name?(full_name) + full_name.count("/") == 2 + end + # A lightweight alternative to `ActiveSupport::Inflector.pluralize`: # Combines `stem` with the `singular` or `plural` suffix based on `count`. # Adds a prefix of the count value if `include_count` is set to true. From 9b3681bef74db2f5cc0b5ef94f7788ee49eb8f25 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 8 Jun 2026 21:02:29 +0100 Subject: [PATCH 3/3] Replace flaky `google-chrome` cask with `firefox` - the `brew bundle` integration test fetches real packages and `google-chrome` repeatedly flakes on Google's download server - `firefox` is an equivalent browser cask served from a more reliable host --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b9d4f33e0e071..138ab0c83001f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -450,7 +450,7 @@ jobs: brew "postgresql@15", restart_service: true cask "rectangle" cask "1password-cli" - cask "google-chrome" + cask "firefox" # VSCode cask is not available on Linux. vscode "shopify.ruby-lsp" if OS.mac? EOS