Skip to content

Commit

Permalink
Add HOMEBREW_SUDO_THROUGH_SUDO_USER
Browse files Browse the repository at this point in the history
This environment variable allows telling Homebrew to use the `SUDO_USER`
variable to `sudo` through that user when Homebrew (Cask) attempts to
run `sudo`.

While we're here, clarify in some messaging that we're running `sudo`
and that that's the password we're asking for; the specific password is
configuration dependent and not the specific password for the user.

Similarly, remove the `Package installers may write to any location`
output; it's kinda spammy and doesn't feel like the right place.
  • Loading branch information
MikeMcQuaid committed Sep 29, 2023
1 parent 5ec560a commit eb1355e
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 6 deletions.
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/artifact/abstract_uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ def uninstall_script(directives, directive_name: :script, force: false, command:
end

def uninstall_pkgutil(*pkgs, command: nil, **_)
ohai "Uninstalling packages; your password may be necessary:"
ohai "Uninstalling packages with sudo; the password may be necessary:"
pkgs.each do |regex|
::Cask::Pkg.all_matching(regex, command).each do |pkg|
puts pkg.package_id
Expand Down
3 changes: 1 addition & 2 deletions Library/Homebrew/cask/artifact/pkg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ def install_phase(**options)
private

def run_installer(command: nil, verbose: false, **_options)
ohai "Running installer for #{cask}; your password may be necessary.",
"Package installers may write to any location; options such as `--appdir` are ignored."
ohai "Running installer for #{cask} with sudo; the password may be necessary."
unless path.exist?
pkg = path.relative_path_from(cask.staged_path)
pkgs = Pathname.glob(cask.staged_path/"**"/"*.pkg").map { |path| path.relative_path_from(cask.staged_path) }
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/staged.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def set_ownership(paths, user: T.must(User.current), group: "staff")
full_paths = remove_nonexistent(paths)
return if full_paths.empty?

ohai "Changing ownership of paths required by #{@cask}; your password may be necessary."
ohai "Changing ownership of paths required by #{@cask} with sudo; the password may be necessary."
@command.run!("/usr/sbin/chown", args: ["-R", "--", "#{user}:#{group}", *full_paths],
sudo: true)
end
Expand Down
5 changes: 5 additions & 0 deletions Library/Homebrew/env_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,11 @@ module EnvConfig
"the system-wide environment file will be loaded last to override any prefix or user settings.",
boolean: true,
},
HOMEBREW_SUDO_THROUGH_SUDO_USER: {
description: "If set, Homebrew will use the `SUDO_USER` environment variable to define the user to " \
"`sudo`(8) through when running `sudo`(8).",
boolean: true,
},
HOMEBREW_TEMP: {
description: "Use this path as the temporary directory for building packages. Changing " \
"this may be needed if your system temporary directory and Homebrew prefix are on " \
Expand Down
3 changes: 3 additions & 0 deletions Library/Homebrew/env_config.rbi
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,9 @@ module Homebrew::EnvConfig
sig { returns(T.nilable(String)) }
def self.sudo_askpass; end

sig { returns(T::Boolean) }
def self.sudo_through_sudo_user?; end

sig { returns(T.nilable(String)) }
def self.svn; end

Expand Down
18 changes: 16 additions & 2 deletions Library/Homebrew/system_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,25 @@ def env_args
set_variables
end

sig { returns(T.nilable(String)) }
def homebrew_sudo_user
ENV.fetch("HOMEBREW_SUDO_USER", nil)

Check warning on line 162 in Library/Homebrew/system_command.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/system_command.rb#L162

Added line #L162 was not covered by tests
end

sig { returns(T::Array[String]) }
def sudo_prefix
user_flags = []
user_flags += ["-u", "root"] if sudo_as_root?
askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : []
user_flags = []
if Homebrew::EnvConfig.sudo_through_sudo_user?
raise ArgumentError, "HOMEBREW_SUDO_THROUGH_SUDO_USER set but SUDO_USER unset!" if homebrew_sudo_user.blank?

user_flags += ["--prompt", "Password for %p:", "-u", homebrew_sudo_user,

Check warning on line 172 in Library/Homebrew/system_command.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/system_command.rb#L172

Added line #L172 was not covered by tests
*askpass_flags,
"-E", *env_args,
"--", "/usr/bin/sudo"]
elsif sudo_as_root?

This comment has been minimized.

Copy link
@Kentzo

Kentzo Dec 19, 2023

Contributor

@MikeMcQuaid I don't think this is the right approach. The idea of sudo_as_root is that some commands must be run as root and root only. No other user, regardless of its group and/or ACLs, can suffice. I think sudo_as_root should take precedence over HOMEBREW_SUDO_THROUGH_SUDO_USER.

This comment has been minimized.

Copy link
@carlocab

carlocab Dec 20, 2023

Member

Please open a pull request (even one that doesn't quite work fully!) that implements your suggestion so we can review it properly.

This comment has been minimized.

Copy link
@Kentzo

Kentzo Dec 20, 2023

Contributor
user_flags += ["-u", "root"]
end
["/usr/bin/sudo", *user_flags, *askpass_flags, "-E", *env_args, "--"]
end

Expand Down
3 changes: 3 additions & 0 deletions docs/Manpage.md
Original file line number Diff line number Diff line change
Expand Up @@ -2362,6 +2362,9 @@ command execution e.g. `$(cat file)`.
- `HOMEBREW_SYSTEM_ENV_TAKES_PRIORITY`
<br>If set in Homebrew's system-wide environment file (`/etc/homebrew/brew.env`), the system-wide environment file will be loaded last to override any prefix or user settings.

- `HOMEBREW_SUDO_THROUGH_SUDO_USER`
<br>If set, Homebrew will use the `SUDO_USER` environment variable to define the user to `sudo`(8) through when running `sudo`(8).

- `HOMEBREW_TEMP`
<br>Use this path as the temporary directory for building packages. Changing this may be needed if your system temporary directory and Homebrew prefix are on different volumes, as macOS has trouble moving symlinks across volumes when the target does not yet exist. This issue typically occurs when using FileVault or custom SSD configurations.

Expand Down
6 changes: 6 additions & 0 deletions manpages/brew.1
Original file line number Diff line number Diff line change
Expand Up @@ -3483,6 +3483,12 @@ Use this as the \fBsvn\fR(1) binary\.
If set in Homebrew\'s system\-wide environment file (\fB/etc/homebrew/brew\.env\fR), the system\-wide environment file will be loaded last to override any prefix or user settings\.
.
.TP
\fBHOMEBREW_SUDO_THROUGH_SUDO_USER\fR
.
.br
If set, Homebrew will use the \fBSUDO_USER\fR environment variable to define the user to \fBsudo\fR(8) through when running \fBsudo\fR(8)\.
.
.TP
\fBHOMEBREW_TEMP\fR
.
.br
Expand Down

0 comments on commit eb1355e

Please sign in to comment.