download: restore artifact-domain fallback for GHCR bottles

Library/Homebrew/download_strategy.rb

@@ -468,13 +468,21 @@ def fetch(timeout: nil)
 
       urls = [url, *mirrors]
 
-      begin
-        url = T.must(urls.shift)
+      if (domain = Homebrew::EnvConfig.artifact_domain)
+        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 = T.must(urls.shift)
 
         ohai "Downloading #{url}"
 
@@ -756,6 +764,35 @@ def resolve_url_basename_time_file_size(url, timeout: nil)
 
     [url, @resolved_basename, nil, nil, nil, false]
   end
+
+  sig { override.params(args: String, options: T.untyped).returns(SystemCommand::Result) }
+  def curl_output(*args, **options)
+    with_github_packages_auth(args) { super }
+  end
+
+  sig {
+    override.params(args: String, print_stdout: T.any(T::Boolean, Symbol), options: T.untyped)
+            .returns(SystemCommand::Result)
+  }
+  def curl(*args, print_stdout: true, **options)
+    with_github_packages_auth(args) { super }
+  end
+
+  sig { params(args: T::Array[String], _block: T.proc.returns(SystemCommand::Result)).returns(SystemCommand::Result) }
+  def with_github_packages_auth(args, &_block)
+    auth_header = "Authorization: #{HOMEBREW_GITHUB_PACKAGES_AUTH}"
+    added_auth_header = false
+    return yield if HOMEBREW_GITHUB_PACKAGES_AUTH.blank?
+    return yield unless args.any? { |arg| arg.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
 
 # Strategy for downloading a file from an Apache Mirror URL.

Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb

@@ -11,6 +11,9 @@
   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(:head_response) do
     <<~HTTP
       HTTP/2 200\r
@@ -27,6 +30,11 @@
   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"))
 
@@ -51,7 +59,7 @@
       expect(strategy).to receive(:system_command)
         .with(
           /curl/,
-          hash_including(args: array_including_cons("--header", "Authorization: Bearer QQ==")),
+          hash_including(args: array_including_cons("--header", "Authorization: #{anonymous_authorization}")),
         )
         .at_least(:once)
         .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
@@ -60,7 +68,7 @@
     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)
@@ -74,5 +82,56 @@
         strategy.fetch
       end
     end
+
+    context "with artifact_domain set" do
+      let(:artifact_domain) { "https://mirror.example.com/oci" }
+
+      it "restores GitHub Packages authentication for ghcr.io requests" do
+        expect(strategy).to receive(:system_command)
+          .with(
+            /curl/,
+            hash_including(
+              args: array_including_cons("--header", "Authorization: #{anonymous_authorization}").and(include(url)),
+            ),
+          )
+          .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
+
+        strategy.send(:curl_output, url)
+      end
+
+      it "does not add GitHub Packages authentication to artifact mirror requests" do
+        mirror_url = url.sub("https://#{GitHubPackages::URL_DOMAIN}", artifact_domain)
+
+        expect(strategy).to receive(:system_command) do |_, options|
+          expect(options[:args]).to include(mirror_url)
+          expect(options[:args]).not_to include("Authorization: #{anonymous_authorization}")
+        end.and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
+
+        strategy.send(:curl_output, mirror_url)
+      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 for artifact mirror requests" do
+          mirror_url = url.sub("https://#{GitHubPackages::URL_DOMAIN}", artifact_domain)
+
+          expect(strategy).to receive(:system_command) do |_, options|
+            expect(options[:args]).to include("--header", "Authorization: #{authorization}", mirror_url)
+          end.and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
+
+          strategy.send(:curl_output, mirror_url)
+          expect(strategy.send(:meta).fetch(:headers)).to include("Authorization: #{authorization}")
+        end
+      end
+    end
   end
 end

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