diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 22059831e8f42..c24d0d420c085 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -468,13 +468,22 @@ def fetch(timeout: nil) urls = [url, *mirrors] - begin - url = T.must(urls.shift) + if (domain = Homebrew::EnvConfig.artifact_domain.presence) + artifact_urls = urls.filter_map do |download_url| + rewritten_url = download_url.sub(%r{^https?://#{GitHubPackages::URL_DOMAIN}/}o, "#{domain.chomp("/")}/") + rewritten_url if rewritten_url != download_url + end - if (domain = Homebrew::EnvConfig.artifact_domain) - url = url.sub(%r{^https?://#{GitHubPackages::URL_DOMAIN}/}o, "#{domain.chomp("/")}/") - urls = [] if Homebrew::EnvConfig.artifact_domain_no_fallback? + urls = if Homebrew::EnvConfig.artifact_domain_no_fallback? + artifact_urls.presence || urls + else + [*artifact_urls, *urls].uniq end + end + + begin + url = urls.fetch(0) + urls = urls.drop(1) ohai "Downloading #{url}" @@ -752,9 +761,37 @@ def initialize(url, name, version, **meta) sig { override.params(url: String, timeout: T.nilable(T.any(Float, Integer))).returns(URLMetadata) } def resolve_url_basename_time_file_size(url, timeout: nil) - return super if @resolved_basename.blank? + with_github_packages_auth(url) do + if @resolved_basename.blank? + super(url, timeout:) + else + [url, @resolved_basename, nil, nil, nil, false] + end + end + end - [url, @resolved_basename, nil, nil, nil, false] + sig { + override.params(url: String, resolved_url: String, timeout: T.nilable(T.any(Float, Integer))) + .returns(T.nilable(SystemCommand::Result)) + } + def _fetch(url:, resolved_url:, timeout:) + with_github_packages_auth(resolved_url) { super } + end + + sig { params(url: String, _block: T.proc.returns(T.untyped)).returns(T.untyped) } + def with_github_packages_auth(url, &_block) + auth_header = "Authorization: #{HOMEBREW_GITHUB_PACKAGES_AUTH}" + added_auth_header = false + return yield if HOMEBREW_GITHUB_PACKAGES_AUTH.blank? + return yield unless url.match?(%r{^https?://#{GitHubPackages::URL_DOMAIN}/}o) + return yield if meta.fetch(:headers, []).include?(auth_header) + + meta[:headers] ||= [] + meta[:headers] << auth_header + added_auth_header = true + yield + ensure + meta[:headers]&.delete(auth_header) if added_auth_header end end diff --git a/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb b/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb index c23c9a6fe5fb8..fb08bb159055a 100644 --- a/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb +++ b/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb @@ -11,6 +11,13 @@ let(:version) { "1.2.3" } let(:specs) { { headers: ["Accept: application/vnd.oci.image.index.v1+json"] } } let(:authorization) { nil } + let(:artifact_domain) { nil } + let(:bearer_prefix) { "Bearer" } + let(:anonymous_authorization) { "#{bearer_prefix} QQ==" } + let(:mirror_url) { url.sub("https://#{GitHubPackages::URL_DOMAIN}", artifact_domain) if artifact_domain.present? } + let(:mirror_download_fails) { false } + let(:stderr) { "curl: (6) Could not resolve host: mirror.example.com" } + let(:curl_requests) { [] } let(:head_response) do <<~HTTP HTTP/2 200\r @@ -24,54 +31,109 @@ HTTP end + def system_command_result(success: true, stdout: "", stderr: "") + status = instance_double(Process::Status, success?: success, exitstatus: success ? 0 : 1, termsig: nil) + output = [] + output << [:stdout, stdout] unless stdout.empty? + output << [:stderr, stderr] unless stderr.empty? + SystemCommand::Result.new(["curl"], output, status, secrets: []) + end + describe "#fetch" do before do stub_const("HOMEBREW_GITHUB_PACKAGES_AUTH", authorization) if authorization.present? + allow(Homebrew::EnvConfig).to receive_messages( + artifact_domain: artifact_domain, + docker_registry_basic_auth_token: nil, + docker_registry_token: nil, + ) allow(strategy).to receive(:curl_version).and_return(Version.new("8.7.1")) - allow(strategy).to receive(:system_command) - .with( - /curl/, - hash_including(args: array_including("--head")), - ) - .twice - .and_return(instance_double( - SystemCommand::Result, - success?: true, - exit_status: instance_double(Process::Status, exitstatus: 0), - stdout: head_response, - )) + allow(strategy).to receive(:system_command) do |_, options| + args = options.fetch(:args) + curl_requests << args + + if args.include?("--head") + system_command_result(stdout: head_response) + elsif mirror_download_fails && mirror_url.present? && args.include?(mirror_url) + system_command_result(success: false, stderr: stderr) + else + system_command_result + end + end strategy.temporary_path.dirname.mkpath FileUtils.touch strategy.temporary_path end it "calls curl with anonymous authentication headers" do - expect(strategy).to receive(:system_command) - .with( - /curl/, - hash_including(args: array_including_cons("--header", "Authorization: Bearer QQ==")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) - strategy.fetch + + ghcr_requests = curl_requests.select { |args| args.include?(url) } + expect(ghcr_requests).not_to be_empty + expect(ghcr_requests).to all(include("Authorization: #{anonymous_authorization}")) end context "with GitHub Packages authentication defined" do - let(:authorization) { "Bearer dead-beef-cafe" } + let(:authorization) { "#{bearer_prefix} dead-beef-cafe" } it "calls curl with the provided header value" do - expect(strategy).to receive(:system_command) - .with( - /curl/, - hash_including(args: array_including_cons("--header", "Authorization: #{authorization}")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + strategy.fetch + + ghcr_requests = curl_requests.select { |args| args.include?(url) } + expect(ghcr_requests).not_to be_empty + expect(ghcr_requests).to all(include("Authorization: #{authorization}")) + end + end + + context "with artifact_domain set" do + let(:artifact_domain) { "https://mirror.example.com/oci" } + it "does not add GitHub Packages authentication to artifact mirror requests" do strategy.fetch + + mirror_requests = curl_requests.select { |args| args.include?(mirror_url) } + expect(mirror_requests).not_to be_empty + expect(mirror_requests).to all(satisfy { |args| !args.include?("Authorization: #{anonymous_authorization}") }) + end + + context "when the artifact mirror download fails" do + let(:mirror_download_fails) { true } + + it "restores GitHub Packages authentication for ghcr.io fallback requests" do + strategy.fetch + + mirror_requests = curl_requests.select { |args| args.include?(mirror_url) } + fallback_requests = curl_requests.select { |args| args.include?(url) } + + expect(mirror_requests).not_to be_empty + expect(fallback_requests).not_to be_empty + expect(mirror_requests).to all(satisfy { |args| !args.include?("Authorization: #{anonymous_authorization}") }) + expect(fallback_requests).to all(include("Authorization: #{anonymous_authorization}")) + end + + context "when authorization is already present in headers" do + let(:authorization) { "#{bearer_prefix} dead-beef-cafe" } + let(:specs) do + { + headers: [ + "Accept: application/vnd.oci.image.index.v1+json", + "Authorization: #{authorization}", + ], + } + end + + it "preserves the existing authorization header across mirror and fallback requests" do + strategy.fetch + + relevant_requests = curl_requests.select { |args| args.include?(mirror_url) || args.include?(url) } + + expect(relevant_requests).not_to be_empty + expect(relevant_requests).to all(include("Authorization: #{authorization}")) + expect(relevant_requests).to all(satisfy { |args| args.count("Authorization: #{authorization}") == 1 }) + end + end end end end diff --git a/Library/Homebrew/test/download_strategies/curl_spec.rb b/Library/Homebrew/test/download_strategies/curl_spec.rb index dea73183e35e2..e585d9a871837 100644 --- a/Library/Homebrew/test/download_strategies/curl_spec.rb +++ b/Library/Homebrew/test/download_strategies/curl_spec.rb @@ -11,6 +11,7 @@ let(:version) { "1.2.3" } let(:specs) { { user: "download:123456" } } let(:artifact_domain) { nil } + let(:artifact_domain_no_fallback) { false } let(:headers) do { "accept-ranges" => "bytes", @@ -29,7 +30,10 @@ describe "#fetch" do before do - allow(Homebrew::EnvConfig).to receive(:artifact_domain).and_return(artifact_domain) + allow(Homebrew::EnvConfig).to receive_messages( + artifact_domain: artifact_domain, + artifact_domain_no_fallback?: artifact_domain_no_fallback, + ) strategy.temporary_path.dirname.mkpath FileUtils.touch strategy.temporary_path @@ -183,19 +187,49 @@ context "with an asset hosted under #{GitHubPackages::URL_DOMAIN} (HTTPS)" do let(:resource_path) { "v2/homebrew/core/spec/manifests/0.0" } let(:url) { "https://#{GitHubPackages::URL_DOMAIN}/#{resource_path}" } + let(:mirror_url) { "#{artifact_domain}/#{resource_path}" } let(:status) { instance_double(Process::Status, success?: true, exitstatus: 0) } + let(:stderr) { "curl: (6) Could not resolve host: mirror.example.com" } it "rewrites the URL correctly" do expect(strategy).to receive(:system_command) .with( /curl/, - hash_including(args: array_including_cons("#{artifact_domain}/#{resource_path}")), + hash_including(args: array_including_cons(mirror_url)), ) .at_least(:once) .and_return(SystemCommand::Result.new(["curl"], [[:stdout, ""]], status, secrets: [])) strategy.fetch end + + it "falls back to the original URL if the artifact mirror download fails" do + expect(strategy).to receive(:_fetch) + .with(url: mirror_url, resolved_url: mirror_url, timeout: anything) + .ordered + .and_raise(ErrorDuringExecution.new(["curl"], status: 1, output: [[:stderr, stderr]])) + expect(strategy).to receive(:_fetch) + .with(url:, resolved_url: url, timeout: anything) + .ordered + .and_return(nil) + + strategy.fetch + end + + context "when artifact_domain_no_fallback is set" do + let(:artifact_domain_no_fallback) { true } + + it "does not fall back to the original URL" do + expect(strategy).to receive(:_fetch) + .with(url: mirror_url, resolved_url: mirror_url, timeout: anything) + .once + .and_raise(ErrorDuringExecution.new(["curl"], status: 1, output: [[:stderr, stderr]])) + + expect do + strategy.fetch + end.to raise_error(CurlDownloadStrategyError, /#{Regexp.escape(mirror_url)}/) + end + end end end end