Skip to content

Commit

Permalink
Merge pull request #17760 from Homebrew/gh-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
nandahkrishna authored Jul 17, 2024
2 parents 9459205 + 6db608f commit 321498c
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 40 deletions.
31 changes: 20 additions & 11 deletions Library/Homebrew/attestation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "date"
require "json"
require "utils/popen"
require "utils/github/api"
require "exceptions"
require "system_command"

Expand Down Expand Up @@ -52,7 +53,6 @@ def self.enabled?
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
Expand All @@ -65,9 +65,25 @@ def self.gh_executable
# 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
ensure_executable!("gh")
end, T.nilable(Pathname))
@gh_executable ||= T.let(nil, T.nilable(Pathname))
return @gh_executable if @gh_executable.present?

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.
Expand Down Expand Up @@ -107,13 +123,6 @@ def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject
# 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

Expand Down
8 changes: 8 additions & 0 deletions Library/Homebrew/cmd/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,14 @@ def run
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 = []
Expand Down
8 changes: 8 additions & 0 deletions Library/Homebrew/cmd/reinstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ def run
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|
Expand Down
8 changes: 8 additions & 0 deletions Library/Homebrew/cmd/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ def run
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

Expand Down
41 changes: 12 additions & 29 deletions Library/Homebrew/test/attestation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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) }
Expand Down Expand Up @@ -68,51 +69,33 @@
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)
.and_return(fake_old_gh)
allow(Formulary).to receive(:factory)
.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
expect(GitHub::API).to receive(:credentials)
.and_return(fake_gh_creds)
it "calls ensure_executable and ensure_formula_installed" do
expect(described_class).to receive(:ensure_executable!)
.with("gh", reason: "verifying attestations")
.and_return(fake_old_gh)

expect(described_class).to receive(:system_command!)
.with(fake_old_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json"],
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(described_class).to receive(:ensure_formula_installed!)
.with("gh", latest: true, reason: "verifying attestations")
.and_return(fake_gh_formula)

expect do
described_class.check_attestation fake_bottle,
described_class::HOMEBREW_CORE_REPO
end.to raise_error(ErrorDuringExecution)
described_class.gh_executable
end
end

describe "::check_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
Expand Down

0 comments on commit 321498c

Please sign in to comment.