Skip to content

Commit

Permalink
Widen attestation verification rollout
Browse files Browse the repository at this point in the history
Take 2 of #17692 but with:

- provide and document `HOMEBREW_NO_VERIFY_ATTESTATIONS`
- don't try to run unless there's GitHub credentials
- don't try to run unless `gh` is installed
- don't try to run in CI

While we're here:
- split out a `Homebrew::EnvConfig.devcmdrun?` helper method
- add some missing `Homebrew::EnvConfig.github_api_token` presence
  checks
  • Loading branch information
MikeMcQuaid committed Jul 14, 2024
1 parent b9da669 commit b8ff4b3
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 11 deletions.
8 changes: 4 additions & 4 deletions Library/Homebrew/attestation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ class GhAuthNeeded < RuntimeError; end
# @api private
sig { returns(T::Boolean) }
def self.enabled?
# TODO: allow this undocumented variable until this is rolled out more
# widely and then we can remove or document it.
return false if ENV.fetch("HOMEBREW_NO_VERIFY_ATTESTATIONS", false)
return false if Homebrew::EnvConfig.no_verify_attestations?
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?

This comment has been minimized.

Copy link
@mmrwoods

mmrwoods Aug 1, 2024

Should this check for gh cli version 2.49.0 or above? The attestation command was only added in that version, so the with the current any version check anyone with an earlier version gets an attestation verification failed error, e.g. from gettext...

attestation verification failed: Failure while executing; `/usr/bin/env GH_TOKEN=****** GH_HOST=github.com /opt/homebrew/bin/gh attestation verify /Users/mwoods/Library/Caches/Homebrew/downloads/7fa27fef64d0e791859cc145b153c3f95043f3803ff4c60e6835bdaaae740589--gettext--0.22.5.arm64_ventura.bottle.tar.gz --repo Homebrew/homebrew-core --format json` exited with 1. Here's the output:
unknown command "attestation" for "gh"

This comment has been minimized.

Copy link
@mmrwoods

mmrwoods Aug 1, 2024

Seems detecting the gh cli version was problematic (#17899), and this will be addressed by adding a message to inform users of the need to upgrade gh cli (#17727). Thanks for the info @woodruffw

This comment has been minimized.

Copy link
@woodruffw

woodruffw Aug 1, 2024

Member

Yep, exactly -- I'll be working on a PR to improve the error message there today.


Homebrew::EnvConfig.developer?
Homebrew::EnvConfig.developer? || Homebrew::EnvConfig.devcmdrun?

Check warning on line 54 in Library/Homebrew/attestation.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/attestation.rb#L54

Added line #L54 was not covered by tests
end

# Returns a path to a suitable `gh` executable for attestation verification.
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/developer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def run
if env_vars.any?
verb = (env_vars.count == 1) ? "is" : "are"
puts "Developer mode is enabled because #{env_vars.to_sentence} #{verb} set."
elsif Homebrew::Settings.read("devcmdrun") == "true"
elsif Homebrew::EnvConfig.devcmdrun?
puts "Developer mode is enabled."
else
puts "Developer mode is disabled."
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ def check_deleted_formula
def check_for_unnecessary_core_tap
return if Homebrew::EnvConfig.developer?
return if Homebrew::EnvConfig.no_install_from_api?
return if Homebrew::Settings.read("devcmdrun") == "true"
return if Homebrew::EnvConfig.devcmdrun?
return unless CoreTap.instance.installed?

<<~EOS
Expand All @@ -879,7 +879,7 @@ def check_for_unnecessary_core_tap
def check_for_unnecessary_cask_tap
return if Homebrew::EnvConfig.developer?
return if Homebrew::EnvConfig.no_install_from_api?
return if Homebrew::Settings.read("devcmdrun") == "true"
return if Homebrew::EnvConfig.devcmdrun?

cask_tap = CoreCaskTap.instance
return unless cask_tap.installed?
Expand Down
10 changes: 10 additions & 0 deletions Library/Homebrew/env_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,11 @@ module EnvConfig
description: "If set, `brew update` will not show the list of newly added formulae/casks.",
boolean: true,
},
HOMEBREW_NO_VERIFY_ATTESTATIONS: {
description: "If set, Homebrew not verify cryptographic attestations of build provenance for bottles " \
"from homebrew-core.",
boolean: true,
},
HOMEBREW_PIP_INDEX_URL: {
description: "If set, `brew install` <formula> will use this URL to download PyPI package resources.",
default_text: "`https://pypi.org/simple`.",
Expand Down Expand Up @@ -556,5 +561,10 @@ def cask_opts_require_sha?
def automatically_set_no_install_from_api?
ENV["HOMEBREW_AUTOMATICALLY_SET_NO_INSTALL_FROM_API"].present?
end

sig { returns(T::Boolean) }
def devcmdrun?
Homebrew::Settings.read("devcmdrun") == "true"
end
end
end
3 changes: 3 additions & 0 deletions Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Library/Homebrew/utils/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def report_command_run(command_instance)
tags = {
command:,
ci: ENV["CI"].present?,
devcmdrun: config_true?(:devcmdrun),
devcmdrun: Homebrew::EnvConfig.devcmdrun?,
developer: Homebrew::EnvConfig.developer?,
}

Expand Down Expand Up @@ -354,7 +354,7 @@ def default_package_tags
prefix:,
default_prefix: Homebrew.default_prefix?,
developer: Homebrew::EnvConfig.developer?,
devcmdrun: config_true?(:devcmdrun),
devcmdrun: Homebrew::EnvConfig.devcmdrun?,
arch: HOMEBREW_PHYSICAL_PROCESSOR,
os: HOMEBREW_SYSTEM,
}
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/utils/curl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def curl_check_http_content(url, url_type, specs: {}, user_agents: [:default], r
check_github_api = url_type == SharedAudits::URL_TYPE_HOMEPAGE &&
details[:status_code] == "404" &&
repo_details &&
Homebrew::EnvConfig.github_api_token
Homebrew::EnvConfig.github_api_token.present?

unless check_github_api
return "The #{url_type} #{url} is not reachable (HTTP status code #{details[:status_code]})"
Expand Down
4 changes: 3 additions & 1 deletion Library/Homebrew/utils/github/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ def self.keychain_username_password
end

def self.credentials
@credentials ||= Homebrew::EnvConfig.github_api_token || github_cli_token || keychain_username_password
@credentials ||= Homebrew::EnvConfig.github_api_token.presence
@credentials ||= github_cli_token.presence
@credentials ||= keychain_username_password.presence
end

sig { returns(Symbol) }
Expand Down

0 comments on commit b8ff4b3

Please sign in to comment.