Merge pull request #17760 from Homebrew/gh-fix

Library/Homebrew/attestation.rb

@@ -4,6 +4,7 @@
 require "date"
 require "json"
 require "utils/popen"
+require "utils/github/api"
 require "exceptions"
 require "system_command"
 
@@ -52,7 +53,6 @@ module Homebrew
       return true if Homebrew::EnvConfig.verify_attestations?
       return false if GitHub::API.credentials.blank?
       return false if ENV.fetch("CI", false)
-      return false unless Formula["gh"].any_version_installed?
 
       Homebrew::EnvConfig.developer? || Homebrew::EnvConfig.devcmdrun?
     end
@@ -65,9 +65,25 @@ module Homebrew
       # NOTE: We set HOMEBREW_NO_VERIFY_ATTESTATIONS when installing `gh` itself,
       #       to prevent a cycle during bootstrapping. This can eventually be resolved
       #       by vendoring a pure-Ruby Sigstore verifier client.
-      @gh_executable ||= T.let(with_env(HOMEBREW_NO_VERIFY_ATTESTATIONS: "1") do
+      @gh_executable ||= T.let(nil, T.nilable(Pathname))
-        ensure_executable!("gh")
+      return @gh_executable if @gh_executable.present?
-      end, T.nilable(Pathname))
+
+      with_env(HOMEBREW_NO_VERIFY_ATTESTATIONS: "1") do
+        @gh_executable = ensure_executable!("gh", reason: "verifying attestations")
+
+        gh_version = Version.new(system_command!(@gh_executable, args: ["--version"], print_stderr: false)
+                                 .stdout.match(/\d+(?:\.\d+)+/i).to_s)
+        if gh_version < GH_ATTESTATION_MIN_VERSION
+          if Formula["gh"].version < GH_ATTESTATION_MIN_VERSION
+            raise "#{@gh_executable} is too old, you must upgrade it to >=#{GH_ATTESTATION_MIN_VERSION} to continue"
+          end
+
+          @gh_executable = ensure_formula_installed!("gh", latest: true,
+                                                           reason: "verifying attestations").opt_bin/"gh"
+        end
+      end
+
+      T.must(@gh_executable)
     end
 
     # Verifies the given bottle against a cryptographic attestation of build provenance.
@@ -107,13 +123,6 @@ module Homebrew
         # Even if we have credentials, they may be invalid or malformed.
         raise GhAuthNeeded, "invalid credentials" if e.status.exitstatus == 4
 
-        gh_version = Version.new(system_command!(gh_executable, args: ["--version"], print_stderr: false)
-                                 .stdout.match(/\d+(?:\.\d+)+/i).to_s)
-        if gh_version < GH_ATTESTATION_MIN_VERSION
-          raise e,
-                "#{gh_executable} is too old, you must upgrade it to continue"
-        end
-
         raise InvalidAttestationError, "attestation verification failed: #{e}"
       end
 

Library/Homebrew/cmd/install.rb

@@ -263,6 +263,14 @@ module Homebrew
           end
         end
 
+        if Homebrew::Attestation.enabled?
+          if formulae.include?(Formula["gh"])
+            formulae.unshift(T.must(formulae.delete(Formula["gh"])))
+          else
+            Homebrew::Attestation.gh_executable
+          end
+        end
+
         # if the user's flags will prevent bottle only-installations when no
         # developer tools are available, we need to stop them early on
         build_flags = []

Library/Homebrew/cmd/reinstall.rb

@@ -124,6 +124,14 @@ module Homebrew
           end
         end
 
+        if Homebrew::Attestation.enabled?
+          if formulae.include?(Formula["gh"])
+            formulae.unshift(T.must(formulae.delete(Formula["gh"])))
+          else
+            Homebrew::Attestation.gh_executable
+          end
+        end
+
         Install.perform_preinstall_checks
 
         formulae.each do |formula|

Library/Homebrew/cmd/upgrade.rb

@@ -134,6 +134,14 @@ module Homebrew
         only_upgrade_formulae = formulae.present? && casks.blank?
         only_upgrade_casks = casks.present? && formulae.blank?
 
+        if Homebrew::Attestation.enabled?
+          if formulae.include?(Formula["gh"])
+            formulae.unshift(formulae.delete(Formula["gh"]))
+          else
+            Homebrew::Attestation.gh_executable
+          end
+        end
+
         upgrade_outdated_formulae(formulae) unless only_upgrade_casks
         upgrade_outdated_casks(casks) unless only_upgrade_formulae
 

Library/Homebrew/test/attestation_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe Homebrew::Attestation do
   let(:fake_gh) { Pathname.new("/extremely/fake/gh") }
   let(:fake_old_gh) { Pathname.new("/extremely/fake/old/gh") }
   let(:fake_gh_creds) { "fake-gh-api-token" }
+  let(:fake_gh_formula) { instance_double(Formula, "gh", opt_bin: Pathname.new("/extremely/fake")) }
   let(:fake_gh_version) { instance_double(SystemCommand::Result, stdout: "2.49.0") }
   let(:fake_old_gh_version) { instance_double(SystemCommand::Result, stdout: "2.48.0") }
   let(:fake_error_status) { instance_double(Process::Status, exitstatus: 1, termsig: nil) }
@@ -68,40 +69,26 @@ RSpec.describe Homebrew::Attestation do
   end
 
   describe "::gh_executable" do
-    it "calls ensure_executable" do
-      expect(described_class).to receive(:ensure_executable!)
-        .with("gh")
-        .and_return(fake_gh)
-
-      described_class.gh_executable
-    end
-  end
-
-  describe "::check_attestation fails with old gh" do
     before do
-      allow(described_class).to receive(:gh_executable)
+      allow(Formulary).to receive(:factory)
-        .and_return(fake_old_gh)
+        .with("gh")
+        .and_return(instance_double(Formula, version: Version.new("2.49.0")))
 
       allow(described_class).to receive(:system_command!)
         .with(fake_old_gh, args: ["--version"], print_stderr: false)
         .and_return(fake_old_gh_version)
     end
 
-    it "raises when gh is too old" do
+    it "calls ensure_executable and ensure_formula_installed" do
-      expect(GitHub::API).to receive(:credentials)
+      expect(described_class).to receive(:ensure_executable!)
-        .and_return(fake_gh_creds)
+        .with("gh", reason: "verifying attestations")
+        .and_return(fake_old_gh)
 
-      expect(described_class).to receive(:system_command!)
+      expect(described_class).to receive(:ensure_formula_installed!)
-        .with(fake_old_gh, args: ["attestation", "verify", cached_download, "--repo",
+        .with("gh", latest: true, reason: "verifying attestations")
-                                  described_class::HOMEBREW_CORE_REPO, "--format", "json"],
+        .and_return(fake_gh_formula)
-              env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds],
-              print_stderr: false, chdir: HOMEBREW_TEMP)
-        .and_raise(ErrorDuringExecution.new(["foo"], status: fake_error_status))
 
-      expect do
+      described_class.gh_executable
-        described_class.check_attestation fake_bottle,
-                                          described_class::HOMEBREW_CORE_REPO
-      end.to raise_error(ErrorDuringExecution)
     end
   end
 
@@ -109,10 +96,6 @@ RSpec.describe Homebrew::Attestation do
     before do
       allow(described_class).to receive(:gh_executable)
         .and_return(fake_gh)
-
-      allow(described_class).to receive(:system_command!)
-        .with(fake_gh, args: ["--version"], print_stderr: false)
-        .and_return(fake_gh_version)
     end
 
     it "raises without any gh credentials" do