Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions Library/Homebrew/bundle/brew.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 && Utils.full_name?(@full_name)

if (tap_with_name = ::Tap.with_formula_name(@full_name))
tap, = tap_with_name
tap.ensure_installed!
Expand Down
6 changes: 5 additions & 1 deletion Library/Homebrew/bundle/cask.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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] && 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"
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions Library/Homebrew/test/bundle/brew_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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!
Expand Down
12 changes: 12 additions & 0 deletions Library/Homebrew/test/bundle/cask_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
14 changes: 14 additions & 0 deletions Library/Homebrew/test/utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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/

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/trust.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions Library/Homebrew/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading