Harden sandboxed install phases

- Deny real home reads unless Homebrew paths live there
- Deny sensitive home paths when full-home denial is skipped
- Run cask completion generation with a fresh fake `HOME`
- Keep cask completion generation offline before running it
- Keep test workflow matrix legs running after failures

Author: MikeMcQuaid

.github/workflows/tests.yml

@@ -186,6 +186,7 @@ jobs:
     needs: syntax
     if: github.event_name != 'push'
     strategy:
+      fail-fast: false
       matrix:
         include:
           - name: update-test (Linux)
@@ -212,6 +213,7 @@ jobs:
     runs-on: ${{ matrix.runs-on }}
     timeout-minutes: 30
     strategy:
+      fail-fast: false
       matrix:
         include:
           - name: tests (online)
@@ -326,6 +328,7 @@ jobs:
     runs-on: ${{ matrix.runs-on }}
     container: ${{ matrix.container }}
     strategy:
+      fail-fast: false
       matrix:
         include:
           - name: test-bot (Linux arm64)
@@ -414,6 +417,7 @@ jobs:
     if: github.repository_owner == 'Homebrew' && github.event_name != 'push'
     runs-on: ${{ matrix.runs-on }}
     strategy:
+      fail-fast: false
       matrix:
         include:
           - name: bundle and services (Linux)
@@ -463,6 +467,7 @@ jobs:
     name: ${{ matrix.name }}
     runs-on: ${{ matrix.runs-on }}
     strategy:
+      fail-fast: false
       matrix:
         include:
           - name: analytics (Linux)

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
 

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

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

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)

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

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|

Library/Homebrew/sandbox.rb

@@ -177,6 +177,63 @@ 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
+    if [
+      HOMEBREW_PREFIX,
+      HOMEBREW_REPOSITORY,
+      HOMEBREW_CACHE,
+      HOMEBREW_TEMP,
+      ENV.fetch("GITHUB_WORKSPACE", nil),
+      ENV.fetch("RUNNER_WORKSPACE", nil),
+      ENV.fetch("RUNNER_TEMP", nil),
+    ].compact.any? do |path|
+      path = Pathname(path)
+      [path.expand_path, path.exist? ? path.realpath : nil].compact.any? { |pathname| pathname.ascend.include?(home) }
+    end
+      [
+        ".ssh",
+        ".aws",
+        ".azure",
+        ".config/gcloud",
+        ".docker",
+        ".gnupg",
+        ".kube",
+        ".netrc",
+        ".npmrc",
+        ".pypirc",
+        ".gem/credentials",
+        "Documents",
+        "Movies",
+        "Music",
+        "Pictures",
+        "Library/Keychains",
+        "Library/Mobile Documents",
+        "Library/CloudStorage",
+        "Dropbox",
+        "Google Drive",
+        "OneDrive",
+      ].each do |path|
+        path = home/path
+        deny_read_path path if path.exist?
+      end
+      return
+    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