Merge pull request #22886 from Homebrew/fix-deferred-env-reexec

Expand deferred env in curl URLs

Author: MikeMcQuaid

Library/Homebrew/context.rb

@@ -9,11 +9,19 @@ module Context
 
   # Struct describing the current execution context.
   class ContextStruct
-    sig { params(debug: T.nilable(T::Boolean), quiet: T.nilable(T::Boolean), verbose: T.nilable(T::Boolean)).void }
-    def initialize(debug: nil, quiet: nil, verbose: nil)
+    sig {
+      params(
+        debug:                          T.nilable(T::Boolean),
+        quiet:                          T.nilable(T::Boolean),
+        verbose:                        T.nilable(T::Boolean),
+        deferred_environment_expansion: T.nilable(T::Boolean),
+      ).void
+    }
+    def initialize(debug: nil, quiet: nil, verbose: nil, deferred_environment_expansion: nil)
       @debug = debug
       @quiet = quiet
       @verbose = verbose
+      @deferred_environment_expansion = deferred_environment_expansion
     end
 
     sig { returns(T::Boolean) }
@@ -30,6 +38,11 @@ def quiet?
     def verbose?
       @verbose == true
     end
+
+    sig { returns(T::Boolean) }
+    def deferred_environment_expansion?
+      @deferred_environment_expansion == true
+    end
   end
 
   @current = T.let(nil, T.nilable(ContextStruct))
@@ -69,13 +82,24 @@ def verbose?
     Context.current.verbose?
   end
 
+  sig { returns(T::Boolean) }
+  def deferred_environment_expansion?
+    Context.current.deferred_environment_expansion?
+  end
+
   sig {
-    params(debug: T.nilable(T::Boolean), quiet: T.nilable(T::Boolean), verbose: T.nilable(T::Boolean),
-           _block: T.proc.void).returns(T.untyped)
+    params(
+      debug:                          T.nilable(T::Boolean),
+      quiet:                          T.nilable(T::Boolean),
+      verbose:                        T.nilable(T::Boolean),
+      deferred_environment_expansion: T.nilable(T::Boolean),
+      _block:                         T.proc.void,
+    ).returns(T.untyped)
   }
-  def with_context(debug: debug?, quiet: quiet?, verbose: verbose?, &_block)
+  def with_context(debug: debug?, quiet: quiet?, verbose: verbose?,
+                   deferred_environment_expansion: deferred_environment_expansion?, &_block)
     old_context = Context.current
-    Thread.current[:context] = ContextStruct.new(debug:, quiet:, verbose:)
+    Thread.current[:context] = ContextStruct.new(debug:, quiet:, verbose:, deferred_environment_expansion:)
 
     begin
       yield

Library/Homebrew/download_strategy.rb

@@ -40,3 +40,14 @@ class Mechanize; end
 require "download_strategy/bazaar_download_strategy"
 require "download_strategy/fossil_download_strategy"
 require "download_strategy/download_strategy_detector"
+
+AbstractDownloadStrategy::HOMEBREW_CONTROLLED_STRATEGIES = T.let([
+  CurlApacheMirrorDownloadStrategy,
+  CurlDownloadStrategy,
+  CurlGitHubPackagesDownloadStrategy,
+  CurlPostDownloadStrategy,
+  HomebrewCurlDownloadStrategy,
+  NoUnzipCurlDownloadStrategy,
+  PyPIDownloadStrategy,
+].freeze, T::Array[T::Class[AbstractDownloadStrategy]])
+AbstractDownloadStrategy.private_constant :HOMEBREW_CONTROLLED_STRATEGIES

Library/Homebrew/download_strategy/abstract_download_strategy.rb

@@ -77,6 +77,11 @@ def quiet?
     Context.current.quiet? || @quiet || false
   end
 
+  sig { params(downloader: AbstractDownloadStrategy).returns(T::Boolean) }
+  def self.expand_deferred_environment_for?(downloader)
+    HOMEBREW_CONTROLLED_STRATEGIES.include?(downloader.class)
+  end
+
   # Unpack {#cached_location} into the current working directory.
   #
   # Additionally, if a block is given, the working directory was previously empty

Library/Homebrew/download_strategy/curl_download_strategy.rb

@@ -16,6 +16,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy
   sig { params(url: String, name: String, version: T.nilable(T.any(String, Version)), meta: T.untyped).void }
   def initialize(url, name, version, **meta)
     @try_partial = T.let(true, T::Boolean)
+    @expand_deferred_environment = T.let(false, T::Boolean)
     @mirrors = T.let(meta.fetch(:mirrors, []), T::Array[String])
     @file_size = T.let(nil, T.nilable(Integer))
     @last_modified = T.let(nil, T.nilable(Time))
@@ -261,6 +262,20 @@ def _curl_download(resolved_url, to, timeout)
     curl_download resolved_url, to:, try_partial: @try_partial, timeout:
   end
 
+  sig { void }
+  def allow_deferred_environment_expansion!
+    @expand_deferred_environment = true
+  end
+
+  sig { params(args: T::Array[String]).returns(T::Array[String]) }
+  def expand_deferred_environment_args(args)
+    return args unless @expand_deferred_environment
+
+    with_context(deferred_environment_expansion: true) do
+      args.map { |arg| ENV.expand_deferred_environment(arg) }
+    end
+  end
+
   # Curl options to be always passed to curl,
   # with raw head calls (`curl --head`) or with actual `fetch`.
   sig { returns(T::Array[String]) }
@@ -273,7 +288,7 @@ def _curl_args
 
     args += ["--user", meta.fetch(:user)] if meta.key?(:user)
 
-    args += meta.fetch(:headers, []).flat_map { |h| ["--header", ENV.expand_deferred_environment(h).strip] }
+    args += expand_deferred_environment_args(meta.fetch(:headers, [])).flat_map { |h| ["--header", h.strip] }
 
     args
   end
@@ -285,7 +300,7 @@ def _curl_opts
 
   sig { override.params(args: String, options: T.untyped).returns(SystemCommand::Result) }
   def curl_output(*args, **options)
-    super(*_curl_args, *args, **_curl_opts, **options)
+    super(*_curl_args, *expand_deferred_environment_args(args), **_curl_opts, **options)
   end
 
   sig {
@@ -294,6 +309,6 @@ def curl_output(*args, **options)
   }
   def curl(*args, print_stdout: true, **options)
     options[:connect_timeout] = 15 unless mirrors.empty?
-    super(*_curl_args, *args, **_curl_opts, **command_output_options, **options)
+    super(*_curl_args, *expand_deferred_environment_args(args), **_curl_opts, **command_output_options, **options)
   end
 end

Library/Homebrew/downloadable.rb

@@ -134,7 +134,11 @@ def downloader
       raise ArgumentError, "attempted to use a `Downloadable` without a URL!" if primary_url.blank?
 
       download_strategy.new(primary_url, download_name, version,
-                            mirrors:, cache:, **T.must(@url).specs)
+                            mirrors:, cache:, **T.must(@url).specs).tap do |downloader|
+        if AbstractDownloadStrategy.expand_deferred_environment_for?(downloader)
+          downloader.send(:allow_deferred_environment_expansion!)
+        end
+      end
     end
   end
 

Library/Homebrew/extend/ENV/sensitive.rb

@@ -2,6 +2,7 @@
 # frozen_string_literal: true
 
 require "env_config"
+require "context"
 
 module EnvSensitive
   extend T::Helpers
@@ -68,6 +69,7 @@ def clear_sensitive_environment_for_eval!(&block)
   sig { params(value: String).returns(String) }
   def expand_deferred_environment(value)
     return value unless value.include?(DEFERRED_PLACEHOLDER_PREFIX)
+    return value unless Context.current.deferred_environment_expansion?
 
     prefix = Regexp.escape(DEFERRED_PLACEHOLDER_PREFIX)
     suffix = Regexp.escape(DEFERRED_PLACEHOLDER_SUFFIX)

Library/Homebrew/test/ENV_spec.rb

@@ -7,6 +7,8 @@
   subject(:env) { {}.extend(EnvActivation).extend(described_class) }
 
   shared_examples EnvActivation do
+    include Context
+
     it "supports switching compilers" do
       subject.clang
       expect(subject["LD"]).to be_nil
@@ -188,20 +190,23 @@
     end
 
     describe "#clear_sensitive_environment_for_eval!" do
-      it "defers HOMEBREW_ secrets to a placeholder that expands to the real value" do
+      it "defers HOMEBREW_ secrets to a placeholder" do
         subject["HOMEBREW_PRIVATE_TOKEN"] = "glpat-secret"
 
         deferred = subject.clear_sensitive_environment_for_eval! { subject["HOMEBREW_PRIVATE_TOKEN"] }
 
         expect(deferred).not_to eq("glpat-secret")
         expect(deferred).not_to be_empty
-        expect(subject.expand_deferred_environment("PRIVATE-TOKEN: #{deferred}")).to eq("PRIVATE-TOKEN: glpat-secret")
+        expect(subject.expand_deferred_environment("PRIVATE-TOKEN: #{deferred}")).to eq("PRIVATE-TOKEN: #{deferred}")
       end
 
       it "never expands a non-HOMEBREW_ secret back to its real value" do
         subject["SECRET_TOKEN"] = "password"
         deferred = subject.clear_sensitive_environment_for_eval! { subject["SECRET_TOKEN"] }
-        expect(subject.expand_deferred_environment("X: #{deferred}")).not_to include("password")
+
+        with_context(deferred_environment_expansion: true) do
+          expect(subject.expand_deferred_environment("X: #{deferred}")).not_to include("password")
+        end
       end
 
       it "keeps HOMEBREW_GITHUB_API_TOKEN readable during eval" do
@@ -222,6 +227,16 @@
       it "leaves values without a deferred placeholder unchanged" do
         expect(subject.expand_deferred_environment("PRIVATE-TOKEN: plain")).to eq("PRIVATE-TOKEN: plain")
       end
+
+      it "expands placeholders only during download strategy fetches" do
+        subject["HOMEBREW_PRIVATE_TOKEN"] = "glpat-secret"
+        deferred = subject.clear_sensitive_environment_for_eval! { subject["HOMEBREW_PRIVATE_TOKEN"] }
+
+        with_context(deferred_environment_expansion: true) do
+          expect(subject.expand_deferred_environment("PRIVATE-TOKEN: #{deferred}"))
+            .to eq("PRIVATE-TOKEN: glpat-secret")
+        end
+      end
     end
   end
 

Library/Homebrew/test/download_strategies/curl_spec.rb

@@ -36,8 +36,35 @@
 
     after { ENV.delete("HOMEBREW_PRIVATE_TOKEN") }
 
-    it "expands the placeholder to the real value at download time" do
-      expect(strategy.send(:_curl_args)).to include("PRIVATE-TOKEN: glpat-secret")
+    it "does not expand the placeholder outside Downloadable#fetch" do
+      expect(header).to include(EnvSensitive::DEFERRED_PLACEHOLDER_PREFIX)
+      expect(strategy.send(:_curl_args)).to include(header)
+    end
+  end
+
+  context "with a deferred HOMEBREW_ secret in the URL" do
+    let(:url) do
+      ENV["HOMEBREW_PRIVATE_TOKEN"] = "glpat-secret"
+      ENV.clear_sensitive_environment_for_eval! do
+        "https://example.com/foo.tar.gz?private_token=#{ENV.fetch("HOMEBREW_PRIVATE_TOKEN", nil)}"
+      end
+    end
+
+    after { ENV.delete("HOMEBREW_PRIVATE_TOKEN") }
+
+    it "does not expand the placeholder outside Downloadable#fetch" do
+      expect(url).to include(EnvSensitive::DEFERRED_PLACEHOLDER_PREFIX)
+      expect(strategy).to receive(:system_command)
+        .with(
+          /curl/,
+          hash_including(args: array_including(url)),
+        )
+        .at_least(:once)
+        .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
+
+      strategy.temporary_path.dirname.mkpath
+      FileUtils.touch strategy.temporary_path
+      strategy.fetch
     end
   end
 

Library/Homebrew/test/resource_spec.rb

@@ -149,6 +149,66 @@
     end
   end
 
+  describe "#fetch" do
+    let(:url) do
+      ENV["HOMEBREW_PRIVATE_TOKEN"] = "glpat-secret"
+      ENV.clear_sensitive_environment_for_eval! do
+        "https://example.com/foo.tar.gz?private_token=#{ENV.fetch("HOMEBREW_PRIVATE_TOKEN", nil)}"
+      end
+    end
+    let(:headers) do
+      {
+        "accept-ranges"  => "bytes",
+        "content-length" => "37182",
+      }
+    end
+
+    before do
+      resource.url(url)
+      allow(resource.downloader).to receive(:curl_headers).with(any_args)
+                                                          .and_return({ responses: [{ headers: }] })
+    end
+
+    after do
+      ENV.delete("HOMEBREW_PRIVATE_TOKEN")
+      resource.clear_cache
+    end
+
+    it "expands deferred environment placeholders while downloading" do
+      expect(url).to include(EnvSensitive::DEFERRED_PLACEHOLDER_PREFIX)
+      expect(resource.downloader).to receive(:system_command)
+        .with(
+          /curl/,
+          hash_including(args: array_including("https://example.com/foo.tar.gz?private_token=glpat-secret")),
+        )
+        .at_least(:once)
+        .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
+
+      resource.downloader.temporary_path.dirname.mkpath
+      FileUtils.touch resource.downloader.temporary_path
+      resource.fetch(verify_download_integrity: false)
+    end
+
+    it "does not expand placeholders for custom curl download strategies" do
+      expect(url).to include(EnvSensitive::DEFERRED_PLACEHOLDER_PREFIX)
+      resource.url(url, using: Class.new(CurlDownloadStrategy))
+      allow(resource.downloader).to receive(:curl_headers).with(any_args)
+                                                          .and_return({ responses: [{ headers: }] })
+
+      expect(resource.downloader).to receive(:system_command)
+        .with(
+          /curl/,
+          hash_including(args: array_including(url)),
+        )
+        .at_least(:once)
+        .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil))
+
+      resource.downloader.temporary_path.dirname.mkpath
+      FileUtils.touch resource.downloader.temporary_path
+      resource.fetch(verify_download_integrity: false)
+    end
+  end
+
   describe "#stage" do
     let(:last_modified) { Time.utc(2026, 5, 6, 13, 43, 5) }
     let(:tarball) { TEST_FIXTURE_DIR/"tarballs/testball-0.1.tbz" }

Library/Homebrew/test/utils/curl_spec.rb

@@ -651,6 +651,22 @@
 
       curl_output("--location", "https://example.com/example.tar.gz")
     end
+
+    it "does not expand deferred environment placeholders" do
+      ENV["HOMEBREW_PRIVATE_TOKEN"] = "glpat-secret"
+      url = ENV.clear_sensitive_environment_for_eval! do
+        "https://example.com/example.tar.gz?private_token=#{ENV.fetch("HOMEBREW_PRIVATE_TOKEN", nil)}"
+      end
+
+      expect(self).to receive(:system_command).with(
+        /curl/,
+        hash_including(args: array_including(url)),
+      ).and_return(
+        instance_double(SystemCommand::Result, success?: true, stdout: ""),
+      )
+
+      curl_output(url)
+    end
   end
 
   describe "::http_status_ok?" do