Merge pull request #20400 from Homebrew/stricter-brew-wrappers

brew.sh: enforce `HOMEBREW_FORCE_BREW_WRAPPER` more strictly

Library/Homebrew/brew.sh

@@ -187,47 +187,10 @@ case "$@" in
     ;;
 esac
 
-# Include some helper functions.
+# Check `HOMEBREW_FORCE_BREW_WRAPPER` for all non-trivial commands
-source "${HOMEBREW_LIBRARY}/Homebrew/utils/helpers.sh"
-
-# Require HOMEBREW_BREW_WRAPPER to be set if HOMEBREW_FORCE_BREW_WRAPPER is set
-# (and HOMEBREW_NO_FORCE_BREW_WRAPPER is not set) for all non-trivial commands
 # (i.e. not defined above this line e.g. formulae or --cellar).
-if [[ -z "${HOMEBREW_NO_FORCE_BREW_WRAPPER:-}" && -n "${HOMEBREW_FORCE_BREW_WRAPPER:-}" ]]
+source "${HOMEBREW_LIBRARY}/Homebrew/utils/wrapper.sh"
-then
+check-brew-wrapper
-  HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW="${HOMEBREW_FORCE_BREW_WRAPPER%/brew}"
-  if [[ -z "${HOMEBREW_BREW_WRAPPER:-}" ]]
-  then
-    odie <<EOS
-conflicting Homebrew wrapper configuration!
-HOMEBREW_FORCE_BREW_WRAPPER was set to ${HOMEBREW_FORCE_BREW_WRAPPER}
-but HOMEBREW_BREW_WRAPPER   was unset.
-
-$(bold "Ensure you run ${HOMEBREW_FORCE_BREW_WRAPPER} directly (not ${HOMEBREW_BREW_FILE})")!
-
-Manually setting your PATH can interfere with Homebrew wrappers.
-Ensure your shell configuration contains:
-  eval "\$(${HOMEBREW_BREW_FILE} shellenv)"
-or that ${HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW} comes before ${HOMEBREW_PREFIX}/bin in your PATH:
-  export PATH="${HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW}:${HOMEBREW_PREFIX}/bin:\$PATH"
-EOS
-  elif [[ "${HOMEBREW_FORCE_BREW_WRAPPER}" != "${HOMEBREW_BREW_WRAPPER}" ]]
-  then
-    odie <<EOS
-conflicting Homebrew wrapper configuration!
-HOMEBREW_FORCE_BREW_WRAPPER was set to ${HOMEBREW_FORCE_BREW_WRAPPER}
-but HOMEBREW_BREW_WRAPPER   was set to ${HOMEBREW_BREW_WRAPPER}
-
-$(bold "Ensure you run ${HOMEBREW_FORCE_BREW_WRAPPER} directly (not ${HOMEBREW_BREW_FILE})")!
-
-Manually setting your PATH can interfere with Homebrew wrappers.
-Ensure your shell configuration contains:
-  eval "\$(${HOMEBREW_BREW_FILE} shellenv)"
-or that ${HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW} comes before ${HOMEBREW_PREFIX}/bin in your PATH:
-  export PATH="${HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW}:${HOMEBREW_PREFIX}/bin:\$PATH"
-EOS
-  fi
-fi
 
 # commands that take a single or no arguments and need to write to HOMEBREW_PREFIX.
 # HOMEBREW_LIBRARY set by bin/brew
@@ -247,6 +210,8 @@ esac
 ##### Next, define all other helper functions.
 #####
 
+source "${HOMEBREW_LIBRARY}/Homebrew/utils/helpers.sh"
+
 check-run-command-as-root() {
   [[ "${EUID}" == 0 || "${UID}" == 0 ]] || return
 

Library/Homebrew/dev-cmd/tests.rb

@@ -245,7 +245,6 @@ module Homebrew
         ENV["HOMEBREW_TEST_GENERIC_OS"] = "1" if args.generic?
         ENV["HOMEBREW_TEST_ONLINE"] = "1" if args.online?
         ENV["HOMEBREW_SORBET_RUNTIME"] = "1"
-        ENV["HOMEBREW_NO_FORCE_BREW_WRAPPER"] = "1"
 
         ENV["USER"] ||= system_command!("id", args: ["-nu"]).stdout.chomp
 

Library/Homebrew/env_config.rb

@@ -89,6 +89,9 @@ module Homebrew
       },
       HOMEBREW_BREW_WRAPPER:                     {
         description: "If set, use wrapper to call `brew` rather than auto-detecting it.",
+        # We use backticks to render "Deprecated:" in bold.
+        # TODO: uncomment line below and remove the line above when odeprecated.
+        # description: "`Deprecated:` If set, use wrapper to call `brew` rather than auto-detecting it.",
       },
       HOMEBREW_BROWSER:                          {
         description:  "Use this as the browser when opening project homepages.",
@@ -264,7 +267,7 @@ module Homebrew
         boolean:     true,
       },
       HOMEBREW_FORCE_BREW_WRAPPER:               {
-        description: "If set, require `$HOMEBREW_BREW_WRAPPER` to be set to the same value as " \
+        description: "If set, require `brew` to be invoked by the value of " \
                      "`$HOMEBREW_FORCE_BREW_WRAPPER` for non-trivial `brew` commands.",
       },
       HOMEBREW_FORCE_VENDOR_RUBY:                {
@@ -396,6 +399,9 @@ module Homebrew
       },
       HOMEBREW_NO_FORCE_BREW_WRAPPER:            {
         description: "If set, disables `$HOMEBREW_FORCE_BREW_WRAPPER` behaviour, even if set.",
+        # We use backticks to render "Deprecated:" in bold.
+        # TODO: uncomment line below and remove the line above when odeprecated.
+        # description: "`Deprecated:` If set, disables `$HOMEBREW_FORCE_BREW_WRAPPER` behaviour, even if set.",
         boolean:     true,
       },
       HOMEBREW_NO_GITHUB_API:                    {
@@ -551,11 +557,24 @@ module Homebrew
     end
 
     CUSTOM_IMPLEMENTATIONS = T.let(Set.new([
+      :HOMEBREW_BREW_WRAPPER,
       :HOMEBREW_MAKE_JOBS,
+      :HOMEBREW_NO_FORCE_BREW_WRAPPER,
       :HOMEBREW_CASK_OPTS,
       :HOMEBREW_FORBID_PACKAGES_FROM_PATHS,
     ]).freeze, T::Set[Symbol])
 
+    FALSY_VALUES = T.let(%w[false no off nil 0].freeze, T::Array[String])
+
+    sig { params(env: String, env_value: T.nilable(String)).void }
+    def check_falsy_values(env, env_value)
+      return unless FALSY_VALUES.include?(env_value&.downcase)
+
+      odisabled "#{env}=#{env_value}", <<~EOS.chomp
+        #{env}=1 to enable and #{env}= (an empty value) to disable
+      EOS
+    end
+
     ENVS.each do |env, hash|
       # Needs a custom implementation.
       next if CUSTOM_IMPLEMENTATIONS.include?(env)
@@ -567,15 +586,10 @@ module Homebrew
         define_method(method_name) do
           env_value = ENV.fetch(env, nil)
 
-          falsy_values = %w[false no off nil 0]
+          check_falsy_values(env, env_value)
-          if falsy_values.include?(env_value&.downcase)
-            odisabled "#{env}=#{env_value}", <<~EOS.chomp
-              #{env}=1 to enable and #{env}= (an empty value) to disable
-            EOS
-          end
 
-          # TODO: Uncomment the remaining part of the line below after the deprecation/disable cycle.
+          # TODO: Uncomment the remaining part of the line below after `check_falsy_values` has been removed.
-          env_value.present? # && !falsy_values.include(env_value.downcase)
+          env_value.present? # && !FALSY_VALUES.include?(env_value.downcase)
         end
       elsif hash[:default].present?
         define_method(method_name) do
@@ -589,6 +603,24 @@ module Homebrew
     end
 
     # Needs a custom implementation.
+    sig { returns(T::Boolean) }
+    def no_force_brew_wrapper?
+      # odeprecated "`HOMEBREW_NO_FORCE_BREW_WRAPPER`"
+      env = "HOMEBREW_NO_FORCE_BREW_WRAPPER"
+      env_value = ENV.fetch(env, nil)
+
+      check_falsy_values(env, env_value)
+
+      # TODO: Uncomment the remaining part of the line below after `check_falsy_values` has been removed.
+      env_value.present? # && !FALSY_VALUES.include?(env_value.downcase)
+    end
+
+    sig { returns(T.nilable(String)) }
+    def brew_wrapper
+      # odeprecated "`HOMEBREW_BREW_WRAPPER`"
+      ENV["HOMEBREW_BREW_WRAPPER"].presence
+    end
+
     sig { returns(String) }
     def make_jobs
       jobs = ENV["HOMEBREW_MAKE_JOBS"].to_i

Library/Homebrew/utils/pid_path.rb

@@ -0,0 +1,34 @@
+#!/usr/bin/env ruby
+# typed: strict
+# frozen_string_literal: true
+
+pid = ARGV[0]&.to_i
+raise "Missing `pid` argument!" unless pid
+
+require "fiddle"
+
+libproc = Fiddle.dlopen("/usr/lib/libproc.dylib")
+
+libproc_proc_pidpath_function = Fiddle::Function.new(
+  libproc["proc_pidpath"],
+  [Fiddle::TYPE_INT, Fiddle::TYPE_VOIDP, Fiddle::TYPE_UINT32_T],
+  Fiddle::TYPE_INT,
+)
+
+# We have to allocate a (char) buffer of exactly `PROC_PIDPATHINFO_MAXSIZE` to use `proc_pidpath`
+# From `include/sys/proc_info.h`, PROC_PIDPATHINFO_MAXSIZE = 4 * MAXPATHLEN
+# From `include/sys/param.h`, MAXPATHLEN = PATH_MAX
+# From `include/sys/syslimits.h`, PATH_MAX = 1024
+# https://github.com/apple-oss-distributions/xnu/blob/e3723e1f17661b24996789d8afc084c0c3303b26/libsyscall/wrappers/libproc/libproc.c#L268-L275
+buffer_size = 4 * 1024 # PROC_PIDPATHINFO_MAXSIZE = 4 * MAXPATHLEN
+buffer = "\0" * buffer_size
+pointer_to_buffer = Fiddle::Pointer.to_ptr(buffer)
+
+# `proc_pidpath` returns a positive value on success. See:
+# https://stackoverflow.com/a/8149198
+# https://github.com/chromium/chromium/blob/86df41504a235f9369f6f53887da12a718a19db4/base/process/process_handle_mac.cc#L37-L44
+# https://github.com/apple-oss-distributions/xnu/blob/e3723e1f17661b24996789d8afc084c0c3303b26/libsyscall/wrappers/libproc/libproc.c#L263-L283
+return_value = libproc_proc_pidpath_function.call(pid, pointer_to_buffer, buffer_size)
+raise "Call to `proc_pidpath` failed! `proc_pidpath` returned #{return_value}." unless return_value.positive?
+
+puts pointer_to_buffer.to_s.strip

Library/Homebrew/utils/wrapper.sh

@@ -0,0 +1,74 @@
+# `brew` wrapper handling helpers.
+
+# HOMEBREW_LIBRARY, HOMEBREW_BREW_FILE, HOMEBREW_ORIGINAL_BREW_FILE, HOMEBREW_PREFIX are set by bin/brew.
+# HOMEBREW_FORCE_BREW_WRAPPER is set by the user environment.
+# shellcheck disable=SC2154
+odie-with-wrapper-message() {
+  source "${HOMEBREW_LIBRARY}/Homebrew/utils/helpers.sh"
+
+  local CUSTOM_MESSAGE="${1}"
+  local HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW="${HOMEBREW_FORCE_BREW_WRAPPER%/brew}"
+
+  odie <<EOS
+conflicting Homebrew wrapper configuration!
+HOMEBREW_FORCE_BREW_WRAPPER was set to ${HOMEBREW_FORCE_BREW_WRAPPER}
+${CUSTOM_MESSAGE}
+
+$(bold "Ensure you run ${HOMEBREW_FORCE_BREW_WRAPPER} directly (not ${HOMEBREW_ORIGINAL_BREW_FILE})")!
+
+Manually setting your PATH can interfere with Homebrew wrappers.
+Ensure your shell configuration contains:
+  eval "\$(${HOMEBREW_BREW_FILE} shellenv)"
+or that ${HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW} comes before ${HOMEBREW_PREFIX}/bin in your PATH:
+  export PATH="${HOMEBREW_FORCE_BREW_WRAPPER_WITHOUT_BREW}:${HOMEBREW_PREFIX}/bin:\$PATH"
+EOS
+}
+
+check-brew-wrapper() {
+  [[ -z "${HOMEBREW_FORCE_BREW_WRAPPER:-}" ]] && return
+  [[ -z "${HOMEBREW_DISABLE_NO_FORCE_BREW_WRAPPER:-}" && -n "${HOMEBREW_NO_FORCE_BREW_WRAPPER:-}" ]] && return
+
+  # Require HOMEBREW_BREW_WRAPPER to be set if HOMEBREW_FORCE_BREW_WRAPPER is set
+  # (and HOMEBREW_NO_FORCE_BREW_WRAPPER and HOMEBREW_DISABLE_NO_FORCE_BREW_WRAPPER are not set).
+  if [[ -z "${HOMEBREW_DISABLE_NO_FORCE_BREW_WRAPPER:-}" && -z "${HOMEBREW_NO_FORCE_BREW_WRAPPER:-}" ]]
+  then
+    if [[ -z "${HOMEBREW_BREW_WRAPPER:-}" ]]
+    then
+      odie-with-wrapper-message "but HOMEBREW_BREW_WRAPPER   was unset."
+    elif [[ "${HOMEBREW_FORCE_BREW_WRAPPER}" != "${HOMEBREW_BREW_WRAPPER}" ]]
+    then
+      odie-with-wrapper-message "but HOMEBREW_BREW_WRAPPER   was set to ${HOMEBREW_BREW_WRAPPER}"
+    fi
+
+    return
+  fi
+
+  # If HOMEBREW_FORCE_BREW_WRAPPER and HOMEBREW_DISABLE_NO_FORCE_BREW_WRAPPER are set,
+  # verify that the path to our parent process is the same as the value of HOMEBREW_FORCE_BREW_WRAPPER,
+  if [[ -n "${HOMEBREW_DISABLE_NO_FORCE_BREW_WRAPPER:-}" ]]
+  then
+    local HOMEBREW_BREW_CALLER HOMEBREW_BREW_CALLER_CHECK_EXIT_CODE
+
+    if [[ -n "${HOMEBREW_MACOS:-}" ]]
+    then
+      source "${HOMEBREW_LIBRARY}/Homebrew/utils/ruby.sh"
+      setup-ruby-path
+      HOMEBREW_BREW_CALLER="$("${HOMEBREW_RUBY_PATH}" "${HOMEBREW_LIBRARY}/Homebrew/utils/pid_path.rb" "${PPID}")"
+    else
+      HOMEBREW_BREW_CALLER="$(readlink -f "/proc/${PPID}/exe")"
+    fi
+    HOMEBREW_BREW_CALLER_CHECK_EXIT_CODE="$?"
+
+    if ((HOMEBREW_BREW_CALLER_CHECK_EXIT_CODE != 0))
+    then
+      # Error message already printed above when populating `HOMEBREW_BREW_CALLER`.
+      odie "failed to check the path to the parent process!"
+    fi
+
+    if [[ "${HOMEBREW_BREW_CALLER:-}" != "${HOMEBREW_FORCE_BREW_WRAPPER}" ]]
+    then
+      source "${HOMEBREW_LIBRARY}/Homebrew/utils/wrapper.sh"
+      odie-with-wrapper-message "but \`brew\` was invoked by ${HOMEBREW_BREW_CALLER}."
+    fi
+  fi
+}

bin/brew

@@ -100,13 +100,6 @@ unset BREW_FILE_DIRECTORY
 # keg_relocate.rb, formula_cellar_checks.rb, and test/global_spec.rb need to change.
 HOMEBREW_LIBRARY="${HOMEBREW_REPOSITORY}/Library"
 
-# Use HOMEBREW_BREW_WRAPPER if set.
-export HOMEBREW_ORIGINAL_BREW_FILE="${HOMEBREW_BREW_FILE}"
-if [[ -n "${HOMEBREW_BREW_WRAPPER:-}" ]]
-then
-  HOMEBREW_BREW_FILE="${HOMEBREW_BREW_WRAPPER}"
-fi
-
 # These variables are exported in this file and are not allowed to be overridden by the user.
 BIN_BREW_EXPORTED_VARS=(
   HOMEBREW_BREW_FILE
@@ -140,6 +133,9 @@ export_homebrew_env_file() {
   done <"${env_file}"
 }
 
+# We only want to be able to set this in `brew.env` files.
+unset HOMEBREW_DISABLE_NO_FORCE_BREW_WRAPPER
+
 # First, load the system-wide configuration.
 export_homebrew_env_file "/etc/homebrew/brew.env"
 
@@ -168,6 +164,13 @@ then
   export_homebrew_env_file "/etc/homebrew/brew.env"
 fi
 
+# Use HOMEBREW_FORCE_BREW_WRAPPER if set.
+export HOMEBREW_ORIGINAL_BREW_FILE="${HOMEBREW_BREW_FILE}"
+if [[ -n "${HOMEBREW_FORCE_BREW_WRAPPER:-}" ]]
+then
+  HOMEBREW_BREW_FILE="${HOMEBREW_FORCE_BREW_WRAPPER}"
+fi
+
 # Copy and export all HOMEBREW_* variables previously mentioned in
 # manpage or used elsewhere by Homebrew.