download: restore artifact-domain fallback for GHCR bottles

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
 

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

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