Harden sandboxed install phases

MikeMcQuaid · MikeMcQuaid · commit 7e152c25c423 · 2026-05-27T08:32:07.000+01:00
- Deny real home reads unless Homebrew paths live there
- Run cask completion generation with a fresh fake `HOME`
- Keep cask completion generation offline before running it
diff --git a/Library/Homebrew/cask/artifact/abstract_artifact.rb b/Library/Homebrew/cask/artifact/abstract_artifact.rb
@@ -5,6 +5,7 @@
 require "env_config"
 require "sandbox"
 require "tempfile"
+require "tmpdir"
 require "utils/output"
 
 module Cask
@@ -199,6 +200,7 @@ def cask_sandbox
         Sandbox.new.tap do |sandbox|
           sandbox.allow_read(path: cask.staged_path, type: :subpath)
           sandbox.allow_write_temp_and_cache
+          sandbox.deny_read_home
           sandbox.deny_all_network
         end
       end
@@ -207,9 +209,11 @@ def cask_sandbox
         params(
           env:  T::Hash[String, T.any(String, T::Boolean, PATH)],
           args: T::Array[T.any(String, Pathname)],
+          home: String,
         ).returns(T::Array[T.any(String, Pathname)])
       }
-      def cask_sandbox_command(env, args)
+      def cask_sandbox_command(env, args, home:)
+        env = { "HOME" => home }.merge(env)
         ["/usr/bin/env", *env.map { |key, value| "#{key}=#{value}" }, *args]
       end
 
diff --git a/Library/Homebrew/cask/artifact/generated_completion.rb b/Library/Homebrew/cask/artifact/generated_completion.rb
@@ -136,19 +136,22 @@ def generate_completion_output(completion_commands, shell_parameter, env)
         end
 
         Tempfile.create("homebrew-cask-completions", HOMEBREW_TEMP) do |output|
-          sandbox.run(
-            *cask_sandbox_command(
-              env,
-              [
-                "/bin/sh",
-                "-c",
-                "output=$1; shift; exec \"$@\" > \"$output\"#{" 2>/dev/null" unless ENV["HOMEBREW_STDERR"]}",
-                "sh",
-                output.path,
-                *(completion_commands + Array(shell_parameter)),
-              ],
-            ),
-          )
+          Dir.mktmpdir("homebrew-cask-home") do |home|
+            sandbox.run(
+              *cask_sandbox_command(
+                env,
+                [
+                  "/bin/sh",
+                  "-c",
+                  "output=$1; shift; exec \"$@\" > \"$output\"#{" 2>/dev/null" unless ENV["HOMEBREW_STDERR"]}",
+                  "sh",
+                  output.path,
+                  *(completion_commands + Array(shell_parameter)),
+                ],
+                home:,
+              ),
+            )
+          end
           output.read
         end
       end
diff --git a/Library/Homebrew/dev-cmd/test.rb b/Library/Homebrew/dev-cmd/test.rb
@@ -91,6 +91,7 @@ def run
               sandbox.allow_write_log(f)
               sandbox.allow_write_xcode
               sandbox.allow_write_path(HOMEBREW_PREFIX/"var/homebrew/locks")
+              sandbox.deny_read_home
               optional_prefix_var_dirs.each do |dir|
                 sandbox.allow_write_path_if_exists HOMEBREW_PREFIX/dir
               end
diff --git a/Library/Homebrew/extend/os/linux/sandbox.rb b/Library/Homebrew/extend/os/linux/sandbox.rb
@@ -281,6 +281,7 @@ def bubblewrap_sandbox_available?(bubblewrap)
       sig { params(args: T.any(String, ::Pathname)).void }
       def run(*args)
         @prepared_writable_paths = T.let([], T.nilable(T::Array[::Pathname]))
+        @masked_read_paths = T.let([], T.nilable(T::Array[::Pathname]))
         old_report_on_exception = T.let(Thread.report_on_exception, T.nilable(T::Boolean))
         Thread.report_on_exception = false
         super
@@ -292,6 +293,8 @@ def run(*args)
           nil
         end
         @prepared_writable_paths = nil
+        @masked_read_paths&.reverse_each { |path| FileUtils.rm_rf(path) }
+        @masked_read_paths = nil
       end
 
       private
@@ -328,6 +331,16 @@ def bubblewrap_args(tmpdir)
           args += ["--ro-bind", path, path]
         end
 
+        denied_read_paths.each do |path|
+          next unless File.exist?(path)
+
+          args += if File.directory?(path)
+            ["--bind", masked_read_path, path]
+          else
+            ["--ro-bind", File::NULL, path]
+          end
+        end
+
         args += ["--bind", tmpdir, tmpdir, "--chdir", tmpdir]
 
         args
@@ -367,6 +380,23 @@ def denied_write_paths
         end.uniq
       end
 
+      sig { returns(T::Array[String]) }
+      def denied_read_paths
+        profile.rules.filter_map do |rule|
+          next if rule.allow || !rule.operation.start_with?("file-read")
+
+          filter = rule.filter
+          filter.path if filter && [:literal, :subpath].include?(filter.type)
+        end.uniq
+      end
+
+      sig { returns(String) }
+      def masked_read_path
+        path = ::Pathname.new(Dir.mktmpdir("homebrew-sandbox-deny-read", HOMEBREW_TEMP))
+        @masked_read_paths&.<< path
+        path.to_s
+      end
+
       sig { params(path: String, type: Symbol).void }
       def prepare_writable_path(path, type)
         pathname = ::Pathname.new(path)
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
@@ -1659,12 +1659,18 @@ def run_post_install
         PATH:          PATH.new(ORIGINAL_PATHS),
       }
 
-      with_env(new_env) do
-        ENV.clear_sensitive_environment!
-        ENV.activate_extensions!
+      Dir.mktmpdir("#{name}-postinstall-") do |home|
+        postinstall_home = Pathname(home)
+        new_env[:HOME] = postinstall_home.to_s
+        setup_home postinstall_home
 
-        with_logging("post_install") do
-          post_install
+        with_env(new_env) do
+          ENV.clear_sensitive_environment!
+          ENV.activate_extensions!
+
+          with_logging("post_install") do
+            post_install
+          end
         end
       end
     ensure
@@ -3245,8 +3251,7 @@ def run_test(keep_tmp: false)
       PATH:          PATH.new(ENV.fetch("PATH"), HOMEBREW_PREFIX/"bin"),
       HOMEBREW_TERM: ENV.fetch("TERM", nil),
       HOMEBREW_PATH: nil,
-    }.merge(common_stage_test_env)
-    test_env[:_JAVA_OPTIONS] += " -Djava.io.tmpdir=#{HOMEBREW_TEMP}"
+    }
 
     ENV.clear_sensitive_environment!
     Utils::Git.set_name_email!
@@ -3255,6 +3260,8 @@ def run_test(keep_tmp: false)
       staging.retain! if keep_tmp
       @testpath = T.let(staging.tmpdir, T.nilable(Pathname))
       test_env[:HOME] = @testpath
+      test_env.merge!(common_stage_test_env(T.must(@testpath)))
+      test_env[:_JAVA_OPTIONS] += " -Djava.io.tmpdir=#{HOMEBREW_TEMP}"
       setup_home T.must(@testpath)
       begin
         with_logging("test") do
@@ -3706,15 +3713,15 @@ def exec_cmd(cmd, args, out, log_filename)
   end
 
   # Common environment variables used at both build and test time.
-  sig { returns(T::Hash[Symbol, String]) }
-  def common_stage_test_env
+  sig { params(home: Pathname).returns(T::Hash[Symbol, String]) }
+  def common_stage_test_env(home)
     {
       _JAVA_OPTIONS:           "-Duser.home=#{HOMEBREW_CACHE}/java_cache",
       GOCACHE:                 "#{HOMEBREW_CACHE}/go_cache",
       GOPATH:                  "#{HOMEBREW_CACHE}/go_mod_cache",
       CARGO_HOME:              "#{HOMEBREW_CACHE}/cargo_cache",
       PIP_CACHE_DIR:           "#{HOMEBREW_CACHE}/pip_cache",
-      CURL_HOME:               ENV.fetch("CURL_HOME") { Dir.home },
+      CURL_HOME:               ENV.fetch("CURL_HOME") { home.to_s },
       PYTHONDONTWRITEBYTECODE: "1",
     }
   end
@@ -3733,7 +3740,7 @@ def stage(interactive: false, debug_symbols: false, &_block)
 
       unless interactive
         stage_env[:HOME] = env_home
-        stage_env.merge!(common_stage_test_env)
+        stage_env.merge!(common_stage_test_env(env_home))
       end
 
       setup_home env_home
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
@@ -1136,7 +1136,11 @@ def build
       sandbox.allow_read_if_exists path: formula_path
       formula.logs.mkpath
       sandbox.record_log(formula.logs/"build.sandbox.log")
-      sandbox.allow_write_path(Dir.home) if interactive?
+      if interactive?
+        sandbox.allow_write_path(Dir.home)
+      else
+        sandbox.deny_read_home
+      end
       sandbox.allow_write_temp_and_cache
       sandbox.allow_write_log(formula)
       sandbox.allow_cvs
@@ -1383,6 +1387,7 @@ def post_install
       sandbox.allow_write_log(formula)
       sandbox.allow_write_xcode
       sandbox.deny_write_homebrew_repository
+      sandbox.deny_read_home
       sandbox.allow_write_cellar(formula)
       sandbox.deny_all_network unless formula.network_access_allowed?(:postinstall)
       Keg.keg_link_directories.each do |dir|
diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb
@@ -177,6 +177,26 @@ def allow_read(path:, type: :literal)
     add_rule allow: true, operation: "file-read*", filter: path_filter(path, type)
   end
 
+  sig { params(path: T.any(String, Pathname), type: Symbol).void }
+  def deny_read(path:, type: :literal)
+    add_rule allow: false, operation: "file-read*", filter: path_filter(path, type)
+  end
+
+  sig { params(path: T.any(String, Pathname)).void }
+  def deny_read_path(path)
+    deny_read path:, type: :subpath
+  end
+
+  sig { void }
+  def deny_read_home
+    home = Pathname(Dir.home(ENV.fetch("USER"))).realpath
+    return if [HOMEBREW_PREFIX, HOMEBREW_REPOSITORY, HOMEBREW_CACHE, HOMEBREW_TEMP].any? do |path|
+      (path.exist? ? path.realpath : path.expand_path).ascend.include?(home)
+    end
+
+    deny_read_path home
+  end
+
   sig { params(path: T.nilable(T.any(String, Pathname)), type: Symbol).void }
   def allow_read_if_exists(path:, type: :literal)
     return unless path
diff --git a/Library/Homebrew/test/cask/artifact/generated_completion_spec.rb b/Library/Homebrew/test/cask/artifact/generated_completion_spec.rb
@@ -40,9 +40,10 @@
         instance_double(Sandbox).tap do |sandbox|
           allow(sandbox).to receive(:allow_read)
           allow(sandbox).to receive(:allow_write_temp_and_cache)
+          allow(sandbox).to receive(:deny_read_home)
           allow(sandbox).to receive(:deny_all_network)
           allow(sandbox).to receive(:run) do |*args|
-            Pathname(args.fetch(6)).write("#{args.fetch(1).delete_prefix("SHELL=")} completion output")
+            Pathname(args.fetch(7)).write("#{args.grep(/^SHELL=/).first.delete_prefix("SHELL=")} completion output")
           end
         end
       end
@@ -57,24 +58,34 @@
       expect((fish_dir/"foo.fish").read).to eq("fish completion output")
     end
 
-    it "sandboxes completion generation" do
+    it "sandboxes completion generation without network access" do
       artifact = cask.artifacts.grep(klass).first
       sandboxes = []
+      calls = []
+      homes = []
 
       allow(Sandbox).to receive_messages(ensure_sandbox_installed!: nil, available?: true)
       allow(Sandbox).to receive(:new) do
         instance_double(Sandbox).tap do |sandbox|
           expect(sandbox).to receive(:allow_read).with(path: staged_path, type: :subpath)
           expect(sandbox).to receive(:allow_write_temp_and_cache)
-          expect(sandbox).to receive(:deny_all_network)
-          allow(sandbox).to receive(:run) { |*args| Pathname(args.fetch(6)).write("completion") }
+          expect(sandbox).to receive(:deny_read_home)
+          expect(sandbox).to receive(:deny_all_network) { calls << :deny_all_network }
+          allow(sandbox).to receive(:run) do |*args|
+            calls << :run
+            homes << Pathname(args.grep(/^HOME=/).first.delete_prefix("HOME="))
+            Pathname(args.fetch(7)).write("completion")
+          end
           sandboxes << sandbox
         end
       end
 
       artifact.install_phase
 
       expect(sandboxes.length).to eq(3)
+      expect(calls).to eq([:deny_all_network, :run, :deny_all_network, :run, :deny_all_network, :run])
+      expect(homes.uniq.length).to eq(3)
+      expect(homes).to all(satisfy { |home| !home.exist? })
     end
 
     it "does not sandbox when HOMEBREW_NO_SANDBOX_CASK is set" do
@@ -101,11 +112,12 @@
           instance_double(Sandbox).tap do |sandbox|
             allow(sandbox).to receive(:allow_read)
             allow(sandbox).to receive(:allow_write_temp_and_cache)
+            allow(sandbox).to receive(:deny_read_home)
             allow(sandbox).to receive(:deny_all_network)
             allow(sandbox).to receive(:run) do |*args|
-              raise "boom" if args.fetch(1) == "SHELL=bash"
+              raise "boom" if args.include?("SHELL=bash")
 
-              Pathname(args.fetch(6)).write("zsh completion")
+              Pathname(args.fetch(7)).write("zsh completion")
             end
           end
         end
@@ -159,18 +171,19 @@
         instance_double(Sandbox).tap do |sandbox|
           allow(sandbox).to receive(:allow_read)
           allow(sandbox).to receive(:allow_write_temp_and_cache)
+          allow(sandbox).to receive(:deny_read_home)
           allow(sandbox).to receive(:deny_all_network)
           allow(sandbox).to receive(:run) do |*args|
             captured_args = args
-            Pathname(args.fetch(6)).write("zsh completion")
+            Pathname(args.fetch(7)).write("zsh completion")
           end
         end
       end
 
       artifact.install_phase
 
       expect(captured_args).to include("--shell=zsh")
-      expect(captured_args.fetch(4)).to end_with(" 2>/dev/null")
+      expect(captured_args.fetch(5)).to end_with(" 2>/dev/null")
       expect(zsh_dir/"_bar").to be_a_file
       expect(bash_dir/"bar").not_to exist
       expect(fish_dir/"bar.fish").not_to exist
diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb
@@ -1008,7 +1008,8 @@ class #{Formulary.class_s(f_name)} < Formula
       allow(Sandbox).to receive_messages(ensure_sandbox_installed!: nil, available?: true, new: sandbox)
       allow(sandbox).to receive_messages(record_log: nil, allow_read_if_exists: nil, allow_write_temp_and_cache: nil,
                                          allow_write_log: nil, allow_cvs: nil, allow_fossil: nil,
-                                         allow_write_xcode: nil, allow_write_cellar: nil, run: nil)
+                                         allow_write_xcode: nil, allow_write_cellar: nil, deny_read_home: nil,
+                                         run: nil)
       allow(formula).to receive_messages(logs: mktmpdir, update_head_version: nil, prefix: mktmpdir,
                                          network_access_allowed?: true)
       allow(Keg).to receive(:new).and_return(instance_double(Keg, empty_installation?: false))
diff --git a/Library/Homebrew/test/sandbox_linux_spec.rb b/Library/Homebrew/test/sandbox_linux_spec.rb
@@ -202,6 +202,14 @@ def expect_sandbox_configuration_command(sandbox_class, assignment, result:)
       expect(args.index("--ro-bind")).to be < args.index("--dev")
     end
 
+    it "masks denied read directories" do
+      sandbox.deny_read_path dir
+
+      bind = args.each_cons(3).find { |arg| arg.fetch(0) == "--bind" && arg.fetch(2) == dir.to_s }
+      expect(bind).not_to be_nil
+      expect(Pathname(bind.fetch(1)).children).to be_empty
+    end
+
     it "overlays Linux runtime filesystems" do
       expect(args.each_cons(2)).to include(["--dev", "/dev"], ["--proc", "/proc"])
     end
diff --git a/Library/Homebrew/test/sandbox_shared_spec.rb b/Library/Homebrew/test/sandbox_shared_spec.rb
