Merge pull request #20655 from Homebrew/copilot/fix-brew-fetch-tap-repo-issue

Fix brew fetch failure with symlinked taps and refactor path validation logic

Library/Homebrew/cask/cask_loader.rb

@@ -6,6 +6,7 @@ require "cask/cask"
 require "uri"
 require "utils/curl"
 require "utils/output"
+require "utils/path"
 require "extend/hash/keys"
 require "api"
 
@@ -112,9 +113,7 @@ module Cask
 
         return unless path.expand_path.exist?
         return if invalid_path?(path)
-
+        return unless ::Utils::Path.loadable_package_path?(path, :cask)
-        return if Homebrew::EnvConfig.forbid_packages_from_paths? &&
-                  !path.realpath.to_s.start_with?("#{Caskroom.path}/", "#{HOMEBREW_LIBRARY}/Taps/")
 
         new(path)
       end

Library/Homebrew/cleanup.rb

@@ -733,7 +733,7 @@ module Homebrew
       formulae_names = removable_formulae.map(&:full_name).sort
 
       verb = dry_run ? "Would autoremove" : "Autoremoving"
-      oh1 "#{verb} #{formulae_names.count} unneeded #{Utils.pluralize("formula", formulae_names.count, plural: "e")}:"
+      oh1 "#{verb} #{formulae_names.count} unneeded #{Utils.pluralize("formula", formulae_names.count)}:"
       puts formulae_names.join("\n")
       return if dry_run
 

Library/Homebrew/cmd/install.rb

@@ -233,8 +233,8 @@ module Homebrew
                                        .map(&:name)
               next if dep_names.blank?
 
-              ohai "Would install #{::Utils.pluralize("dependenc", dep_names.count, plural: "ies", singular: "y",
+              ohai "Would install #{::Utils.pluralize("dependency", dep_names.count, include_count: true)} " \
-                                                  include_count: true)} for #{cask.full_name}:"
+                   "for #{cask.full_name}:"
               puts dep_names.join(" ")
             end
             return

Library/Homebrew/cmd/tap-info.rb

@@ -57,7 +57,7 @@ module Homebrew
           end
           info = Utils.pluralize("tap", tap_count, include_count: true)
           info += ", #{private_count} private"
-          info += ", #{Utils.pluralize("formula", formula_count, plural: "e", include_count: true)}"
+          info += ", #{Utils.pluralize("formula", formula_count, include_count: true)}"
           info += ", #{Utils.pluralize("command", command_count, include_count: true)}"
           info += ", #{HOMEBREW_TAP_DIRECTORY.dup.abv}" if HOMEBREW_TAP_DIRECTORY.directory?
           puts info

Library/Homebrew/cmd/update-report.rb

@@ -842,7 +842,7 @@ class ReporterHub
     msg = ""
 
     if outdated_formulae.positive?
-      noun = Utils.pluralize("formula", outdated_formulae, plural: "e")
+      noun = Utils.pluralize("formula", outdated_formulae)
       msg += "#{Tty.bold}#{outdated_formulae}#{Tty.reset} outdated #{noun}"
     end
 

Library/Homebrew/dev-cmd/audit.rb

@@ -294,9 +294,7 @@ module Homebrew
           errors_summary = Utils.pluralize("problem", total_problems_count, include_count: true)
 
           error_sources = []
-          if formula_count.positive?
+          error_sources << Utils.pluralize("formula", formula_count, include_count: true) if formula_count.positive?
-            error_sources << Utils.pluralize("formula", formula_count, plural: "e", include_count: true)
-          end
           error_sources << Utils.pluralize("cask", cask_count, include_count: true) if cask_count.positive?
           error_sources << Utils.pluralize("tap", tap_count, include_count: true) if tap_count.positive?
 

Library/Homebrew/exceptions.rb

@@ -579,7 +579,7 @@ class UnbottledError < RuntimeError
     require "utils"
 
     msg = <<~EOS
-      The following #{Utils.pluralize("formula", formulae.count, plural: "e")} cannot be installed from #{Utils.pluralize("bottle", formulae.count)} and must be
+      The following #{Utils.pluralize("formula", formulae.count)} cannot be installed from #{Utils.pluralize("bottle", formulae.count)} and must be
       built from source.
         #{formulae.to_sentence}
     EOS

Library/Homebrew/formulary.rb

@@ -7,6 +7,7 @@ require "tab"
 require "utils"
 require "utils/bottles"
 require "utils/output"
+require "utils/path"
 require "service"
 require "utils/curl"
 require "deprecate_disable"
@@ -727,29 +728,7 @@ module Formulary
       end
 
       return unless path.expand_path.exist?
-
+      return unless ::Utils::Path.loadable_package_path?(path, :formula)
-      if Homebrew::EnvConfig.forbid_packages_from_paths?
-        path_realpath = path.realpath.to_s
-        path_string = path.to_s
-        if (path_realpath.end_with?(".rb") || path_string.end_with?(".rb")) &&
-           !path_realpath.start_with?("#{HOMEBREW_CELLAR}/", "#{HOMEBREW_LIBRARY}/Taps/") &&
-           !path_string.start_with?("#{HOMEBREW_CELLAR}/", "#{HOMEBREW_LIBRARY}/Taps/")
-          if path_string.include?("./") || path_string.end_with?(".rb") || path_string.count("/") != 2
-            raise <<~WARNING
-              Homebrew requires formulae to be in a tap, rejecting:
-                #{path_string} (#{path_realpath})
-
-              To create a tap, run e.g.
-                brew tap-new <user|org>/<repository>
-              To create a formula in a tap run e.g.
-                brew create <url> --tap=<user|org>/<repository>
-            WARNING
-          elsif path_string.count("/") == 2
-            # Looks like a tap, let's quietly return but not error.
-            return
-          end
-        end
-      end
 
       if (tap = Tap.from_path(path))
         # Only treat symlinks in taps as aliases.

Library/Homebrew/install.rb

@@ -405,8 +405,7 @@ module Homebrew
         return if formulae_names_to_install.empty?
 
         if dry_run
-          ohai "Would install #{Utils.pluralize("formula", formulae_names_to_install.count,
+          ohai "Would install #{Utils.pluralize("formula", formulae_names_to_install.count, include_count: true)}:"
-                                                plural: "e", include_count: true)}:"
           puts formulae_names_to_install.join(" ")
 
           formula_installers.each do |fi|
@@ -429,8 +428,8 @@ module Homebrew
       def print_dry_run_dependencies(formula, dependencies)
         return if dependencies.empty?
 
-        ohai "Would install #{Utils.pluralize("dependenc", dependencies.count, plural: "ies", singular: "y",
+        ohai "Would install #{Utils.pluralize("dependency", dependencies.count, include_count: true)} " \
-                                            include_count: true)} for #{formula.name}:"
+             "for #{formula.name}:"
         formula_names = dependencies.map { |(dep, _options)| yield dep.to_formula }
         puts formula_names.join(" ")
       end
@@ -446,7 +445,7 @@ module Homebrew
 
         sizes = compute_total_sizes(formulae, debug: args.debug?)
 
-        puts "#{::Utils.pluralize("Formula", formulae.count, plural: "e")} \
+        puts "#{::Utils.pluralize("Formula", formulae.count)} \
 (#{formulae.count}): #{formulae.join(", ")}\n\n"
         puts "Download Size: #{disk_usage_readable(sizes.fetch(:download))}"
         puts "Install Size:  #{disk_usage_readable(sizes.fetch(:installed))}"

Library/Homebrew/retryable_download.rb

@@ -93,7 +93,7 @@ module Homebrew
 
       wait = 2 ** @try
       unless quiet
-        what = Utils.pluralize("tr", tries_remaining, plural: "ies", singular: "y")
+        what = Utils.pluralize("try", tries_remaining)
         ohai "Retrying download in #{wait}s... (#{tries_remaining} #{what} left)"
       end
       sleep wait

Library/Homebrew/search.rb

@@ -34,7 +34,7 @@ module Homebrew
           unofficial = Tap.all.sum { |tap| tap.official? ? 0 : tap.formula_files.size }
           if unofficial.positive?
             opoo "Use `--eval-all` to search #{unofficial} additional " \
-                 "#{Utils.pluralize("formula", unofficial, plural: "e")} in third party taps."
+                 "#{Utils.pluralize("formula", unofficial)} in third party taps."
           end
           descriptions = Homebrew::API::Formula.all_formulae.transform_values { |data| data["desc"] }
           Descriptions.search(string_or_regex, search_type, descriptions, eval_all, cache_store_hash: true).print

Library/Homebrew/tap.rb

@@ -738,7 +738,7 @@ class Tap
     end
 
     if (formula_count = formula_files.count).positive?
-      contents << Utils.pluralize("formula", formula_count, plural: "e", include_count: true)
+      contents << Utils.pluralize("formula", formula_count, include_count: true)
     end
 
     contents

Library/Homebrew/test/cask/cask_loader_spec.rb

@@ -236,4 +236,73 @@ RSpec.describe Cask::CaskLoader, :cask do
       expect(described_class.load_prefer_installed("test-cask").tap).to eq(foo_tap)
     end
   end
+
+  describe "FromPathLoader with symlinked taps" do
+    let(:cask_token) { "testcask" }
+    let(:tmpdir) { mktmpdir }
+    let(:real_tap_path) { tmpdir / "real_tap" }
+    let(:homebrew_prefix) { tmpdir / "homebrew" }
+    let(:taps_dir) { homebrew_prefix / "Library" / "Taps" / "testuser" }
+    let(:symlinked_tap_path) { taps_dir / "homebrew-testtap" }
+    let(:cask_file_path) { symlinked_tap_path / "Casks" / "#{cask_token}.rb" }
+    let(:cask_content) do
+      <<~RUBY
+        cask "#{cask_token}" do
+          version "1.0.0"
+          sha256 "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
+
+          url "https://example.com/#{cask_token}-\#{version}.dmg"
+          name "Test Cask"
+          desc "A test cask for symlink testing"
+          homepage "https://example.com"
+
+          app "TestCask.app"
+        end
+      RUBY
+    end
+
+    after do
+      tmpdir.rmtree if tmpdir.exist?
+    end
+
+    before do
+      # Create real tap directory structure
+      (real_tap_path / "Casks").mkpath
+      (real_tap_path / "Casks" / "#{cask_token}.rb").write(cask_content)
+
+      # Create homebrew prefix structure
+      taps_dir.mkpath
+
+      # Create symlink to the tap (this simulates what setup-homebrew does)
+      symlinked_tap_path.make_symlink(real_tap_path)
+
+      # Set HOMEBREW_LIBRARY to our test prefix for the security check
+      stub_const("HOMEBREW_LIBRARY", homebrew_prefix / "Library")
+      allow(Homebrew::EnvConfig).to receive(:forbid_packages_from_paths?).and_return(true)
+    end
+
+    context "when HOMEBREW_FORBID_PACKAGES_FROM_PATHS is enabled" do
+      it "allows loading casks from symlinked taps" do
+        loader = Cask::CaskLoader::FromPathLoader.try_new(cask_file_path)
+        expect(loader).not_to be_nil
+        expect(loader).to be_a(Cask::CaskLoader::FromPathLoader)
+
+        cask = loader.load(config: nil)
+        expect(cask.token).to eq(cask_token)
+        expect(cask.version).to eq(Version.new("1.0.0"))
+      end
+    end
+
+    context "when HOMEBREW_FORBID_PACKAGES_FROM_PATHS is disabled" do
+      before do
+        allow(Homebrew::EnvConfig).to receive(:forbid_packages_from_paths?).and_return(false)
+      end
+
+      it "allows loading casks from symlinked taps" do
+        loader = Cask::CaskLoader::FromPathLoader.try_new(cask_file_path)
+        expect(loader).not_to be_nil
+        expect(loader).to be_a(Cask::CaskLoader::FromPathLoader)
+      end
+    end
+  end
 end

Library/Homebrew/upgrade.rb

@@ -251,7 +251,7 @@ module Homebrew
           ohai "No outdated dependents to upgrade!" unless dry_run
         else
           installed_formulae = (dry_run ? formulae : FormulaInstaller.installed.to_a).dup
-          formula_plural = Utils.pluralize("formula", installed_formulae.count, plural: "e")
+          formula_plural = Utils.pluralize("formula", installed_formulae.count)
           upgrade_verb = dry_run ? "Would upgrade" : "Upgrading"
           ohai "#{upgrade_verb} #{Utils.pluralize("dependent", upgradeable.count,
                                                   include_count: true)} of upgraded #{formula_plural}:"

Library/Homebrew/utils.rb

@@ -160,6 +160,15 @@ module Utils
     params(stem: String, count: Integer, plural: String, singular: String, include_count: T::Boolean).returns(String)
   }
   def self.pluralize(stem, count, plural: "s", singular: "", include_count: false)
+    case stem
+    when "formula"
+      plural = "e"
+    when "dependency", "try"
+      stem = stem.delete_suffix("y")
+      plural = "ies"
+      singular = "y"
+    end
+
     prefix = include_count ? "#{count} " : ""
     suffix = (count == 1) ? singular : plural
     "#{prefix}#{stem}#{suffix}"

Library/Homebrew/utils/path.rb

@@ -10,5 +10,44 @@ module Utils
       child_pathname.ascend { |p| return true if p == parent_pathname }
       false
     end
+
+    sig { params(path: Pathname, package_type: Symbol).returns(T::Boolean) }
+    def self.loadable_package_path?(path, package_type)
+      return true unless Homebrew::EnvConfig.forbid_packages_from_paths?
+
+      path_realpath = path.realpath.to_s
+      path_string = path.to_s
+
+      allowed_paths = ["#{HOMEBREW_LIBRARY}/Taps/"]
+      allowed_paths << if package_type == :formula
+        "#{HOMEBREW_CELLAR}/"
+      else
+        "#{Cask::Caskroom.path}/"
+      end
+
+      return true if !path_realpath.end_with?(".rb") && !path_string.end_with?(".rb")
+      return true if allowed_paths.any? { |path| path_realpath.start_with?(path) }
+      return true if allowed_paths.any? { |path| path_string.start_with?(path) }
+
+      # Looks like a local path, Ruby file and not a tap.
+      if path_string.include?("./") || path_string.end_with?(".rb") || path_string.count("/") != 2
+        package_type_plural = Utils.pluralize(package_type.to_s, 2)
+        path_realpath_if_different = " (#{path_realpath})" if path_realpath != path_string
+        create_flag = " --cask" if package_type == :cask
+
+        raise <<~WARNING
+          Homebrew requires #{package_type_plural} to be in a tap, rejecting:
+            #{path_string}#{path_realpath_if_different}
+
+          To create a tap, run e.g.
+            brew tap-new <user|org>/<repository>
+          To create a #{package_type} in a tap run e.g.
+            brew create#{create_flag} <url> --tap=<user|org>/<repository>
+        WARNING
+      else
+        # Looks like a tap, let's quietly reject but not error.
+        path_string.count("/") != 2
+      end
+    end
   end
 end