Merge pull request #22384 from Homebrew/filter-env-ruby-loading

Filter sensitive environment variables during Ruby evals

Author: MikeMcQuaid

AGENTS.md

@@ -58,5 +58,6 @@ When running Ruby directly (e.g. `ruby -e ...`, `gem`, profiling tools), never u
 8. Prefer shelling out via `HOMEBREW_BREW_FILE` instead of requiring `cmd/` or `dev-cmd` when composing brew commands.
 9. Inline new or existing methods as methods or local variables unless they are reused 2+ times or needed for unit tests.
 10. Avoid `T.must`; prefer explicit nil checks or APIs that return non-nil values.
-11. Keep `extend/os/*` prepends as thin as possible; put the `prepend` in the OS-specific `linux` or `macos` file rather than the shared `extend/os/*` loader with an inline `if`, and prefer putting substantive logic in shared code outside `extend/` when practical so it can be tested on all platforms instead of relying on `:needs_linux` or `:needs_macos` specs.
-12. When Bash logic mirrors Ruby logic, keep both implementations in sync and add two-way comments naming the matching Ruby and Bash locations; keep matching helper filenames aligned where practical.
+11. Avoid `T.unsafe(self)` whenever possible; prefer `requires_ancestor` or similar typed module patterns.
+12. Keep `extend/os/*` prepends as thin as possible; put the `prepend` in the OS-specific `linux` or `macos` file rather than the shared `extend/os/*` loader with an inline `if`, and prefer putting substantive logic in shared code outside `extend/` when practical so it can be tested on all platforms instead of relying on `:needs_linux` or `:needs_macos` specs.
+13. When Bash logic mirrors Ruby logic, keep both implementations in sync and add two-way comments naming the matching Ruby and Bash locations; keep matching helper filenames aligned where practical.

Library/Homebrew/cask/cask_loader.rb

@@ -8,6 +8,7 @@
 require "utils/output"
 require "utils/path"
 require "extend/hash/keys"
+require "extend/ENV/sensitive"
 require "api"
 
 module Cask
@@ -102,7 +103,9 @@ def initialize(content, tap: T.unsafe(nil))
       def load(config:)
         @config = config
 
-        instance_eval(content, __FILE__, __LINE__)
+        ENV.clear_sensitive_environment! do
+          instance_eval(content, __FILE__, __LINE__)
+        end
       end
     end
 
@@ -187,8 +190,10 @@ def load(config:)
         end
 
         begin
-          instance_eval(content, path.to_s).tap do |cask|
-            raise CaskUnreadableError.new(token, "'#{path}' does not contain a cask.") unless cask.is_a?(Cask)
+          ENV.clear_sensitive_environment! do
+            instance_eval(content, path.to_s).tap do |cask|
+              raise CaskUnreadableError.new(token, "'#{path}' does not contain a cask.") unless cask.is_a?(Cask)
+            end
           end
         rescue NameError, ArgumentError, ScriptError => e
           error = CaskUnreadableError.new(token, e.message)

Library/Homebrew/cli/parser.rb

@@ -7,6 +7,7 @@
 require "cli/args"
 require "cli/error"
 require "commands"
+require "extend/ENV/sensitive"
 require "optparse"
 require "utils/tty"
 require "utils/formatter"
@@ -57,7 +58,7 @@ def self.from_cmd_path(cmd_path)
         cmd_name = cmd_args_method_name.to_s.delete_suffix("_args").tr("_", "-")
 
         begin
-          if Homebrew.require?(cmd_path)
+          if ENV.clear_sensitive_environment! { Homebrew.require?(cmd_path) }
             cmd = Homebrew::AbstractCommand.command(cmd_name)
             if cmd
               cmd.parser

Library/Homebrew/commands.rb

@@ -3,6 +3,7 @@
 
 require "homebrew"
 require "cli/parser"
+require "extend/ENV/sensitive"
 
 # Helper functions for commands.
 module Commands
@@ -88,7 +89,7 @@ def self.internal_dev_cmd_path(cmd)
   sig { params(cmd: String).returns(T.nilable(Pathname)) }
   def self.external_ruby_v2_cmd_path(cmd)
     path = which("#{cmd}.rb", tap_cmd_directories)
-    path if Homebrew.require?(path)
+    path if ENV.clear_sensitive_environment! { Homebrew.require?(path) }
   end
 
   # Ruby commands which are run by being `require`d.

Library/Homebrew/extend/ENV.rb

@@ -3,6 +3,7 @@
 
 require "hardware"
 require "diagnostic"
+require "extend/ENV/sensitive"
 require "extend/ENV/shared"
 require "extend/ENV/std"
 require "extend/ENV/super"
@@ -18,6 +19,8 @@
 # <!-- vale on -->
 
 module EnvActivation
+  include EnvSensitive
+
   sig { params(env: T.nilable(String)).void }
   def activate_extensions!(env: nil)
     if superenv?(env)
@@ -52,21 +55,6 @@ def with_build_environment(env: nil, cc: nil, build_bottle: false, bottle_arch:
       replace(old_env)
     end
   end
-
-  sig { params(key: T.any(String, Symbol)).returns(T::Boolean) }
-  def sensitive?(key)
-    key.match?(/(cookie|key|token|password|passphrase|auth)/i)
-  end
-
-  sig { returns(T::Hash[String, String]) }
-  def sensitive_environment
-    select { |key, _| sensitive?(key) }
-  end
-
-  sig { void }
-  def clear_sensitive_environment!
-    each_key { |key| delete key if sensitive?(key) }
-  end
 end
 
 ENV.extend(EnvActivation)

Library/Homebrew/extend/ENV.rbi

@@ -1,6 +1,9 @@
 # typed: strict
 
+module EnvSensitive; end
+
 module EnvActivation
+  include EnvSensitive
   include SharedEnvExtension
 end
 

Library/Homebrew/extend/ENV/sensitive.rb

@@ -0,0 +1,36 @@
+# typed: strict
+# frozen_string_literal: true
+
+module EnvSensitive
+  extend T::Helpers
+
+  requires_ancestor { Sorbet::Private::Static::ENVClass }
+
+  sig { params(key: T.any(String, Symbol)).returns(T::Boolean) }
+  def sensitive?(key)
+    key.match?(/(cookie|key|token|password|passphrase|auth)/i)
+  end
+
+  sig { returns(T::Hash[String, String]) }
+  def sensitive_environment
+    select { |key, _| sensitive?(key) }
+  end
+
+  sig { params(block: T.nilable(T.proc.returns(T.untyped))).returns(T.untyped) }
+  def clear_sensitive_environment!(&block)
+    unless block
+      each_key { |key| delete key if sensitive?(key) }
+      return
+    end
+
+    old_env = to_hash.dup
+    begin
+      clear_sensitive_environment!
+      yield
+    ensure
+      replace(old_env)
+    end
+  end
+end
+
+ENV.extend(EnvSensitive)

Library/Homebrew/formulary.rb

@@ -13,6 +13,7 @@
 require "utils/curl"
 require "extend/hash/deep_transform_values"
 require "extend/hash/keys"
+require "extend/ENV/sensitive"
 require "tap"
 
 # The {Formulary} is responsible for creating instances of {Formula}.
@@ -151,10 +152,12 @@ def self.load_formula(name, path, contents, namespace, flags:, ignore_errors:)
         raise FormulaUnreadableError.new(name, e)
       end
     end
-    if ignore_errors
-      Ignorable.hook_raise(&eval_formula)
-    else
-      eval_formula.call
+    ENV.clear_sensitive_environment! do
+      if ignore_errors
+        Ignorable.hook_raise(&eval_formula)
+      else
+        eval_formula.call
+      end
     end
 
     class_name = class_s(name)

Library/Homebrew/test/ENV_spec.rb

@@ -162,6 +162,23 @@
         subject.clear_sensitive_environment!
         expect(subject["FOO"]).to eq "bar"
       end
+
+      it "restores the environment after yielding" do
+        subject["SECRET_TOKEN"] = "password"
+        subject["FOO"] = "bar"
+
+        result = subject.clear_sensitive_environment! do
+          subject["FOO"] = "baz"
+          subject["OTHER_TOKEN"] = "secret"
+
+          [subject["SECRET_TOKEN"], subject["FOO"]]
+        end
+
+        expect(result).to eq([nil, "baz"])
+        expect(subject["SECRET_TOKEN"]).to eq("password")
+        expect(subject["FOO"]).to eq("bar")
+        expect(subject).not_to include("OTHER_TOKEN")
+      end
     end
   end
 

Library/Homebrew/test/cask/cask_loader_spec.rb

@@ -254,6 +254,31 @@
   end
 
   describe "FromPathLoader" do
+    it "clears sensitive environment variables while evaluating casks" do
+      cask_token = "sensitive-env"
+      cask_file = mktmpdir/"#{cask_token}.rb"
+      cask_file.write <<~RUBY
+        cask "#{cask_token}" do
+          version "1.0.0"
+          sha256 "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
+
+          url "https://example.com/app.dmg"
+          name "Sensitive Env"
+          desc ENV.key?("SECRET_TOKEN") ? "Secret present" : "Secret absent"
+          homepage "https://example.com"
+
+          app "App.app"
+        end
+      RUBY
+
+      with_env(SECRET_TOKEN: "password") do
+        cask = Cask::CaskLoader::FromPathLoader.new(cask_file).load(config: nil)
+
+        expect(cask.desc).to eq("Secret absent")
+        expect(ENV.fetch("SECRET_TOKEN", nil)).to eq("password")
+      end
+    end
+
     describe "loading a cask with a removed DSL method" do
       let(:tmpdir) { mktmpdir }
       let(:cask_token) { "removed-method-cask" }