diff --git a/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb b/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb index b4bdf3de66..0fc9eac2bb 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb @@ -50,7 +50,13 @@ module Hbc end with_choices_file do |choices_path| args << "-applyChoiceChangesXML" << choices_path if choices_path - command.run!("/usr/sbin/installer", sudo: true, args: args, print_stdout: true) + logged_in_user = Utils.current_user + env = { + "LOGNAME" => logged_in_user, + "USER" => logged_in_user, + "USERNAME" => logged_in_user, + } + command.run!("/usr/sbin/installer", sudo: true, args: args, print_stdout: true, env: env) end end diff --git a/Library/Homebrew/cask/lib/hbc/system_command.rb b/Library/Homebrew/cask/lib/hbc/system_command.rb index 0c6f0eb367..bea1286aee 100644 --- a/Library/Homebrew/cask/lib/hbc/system_command.rb +++ b/Library/Homebrew/cask/lib/hbc/system_command.rb @@ -37,7 +37,7 @@ module Hbc result end - def initialize(executable, args: [], sudo: false, input: [], print_stdout: false, print_stderr: true, must_succeed: false, path: ENV["PATH"], **options) + def initialize(executable, args: [], sudo: false, input: [], print_stdout: false, print_stderr: true, must_succeed: false, env: {}, **options) @executable = executable @args = args @sudo = sudo @@ -47,7 +47,11 @@ module Hbc @must_succeed = must_succeed options.extend(HashValidator).assert_valid_keys(:chdir) @options = options - @path = path + @env = { "PATH" => ENV["PATH"] }.merge(env) + + @env.keys.grep_v(/^[\w&&\D]\w*$/) do |name| + raise ArgumentError, "Invalid variable name: '#{name}'" + end end def command @@ -56,14 +60,22 @@ module Hbc private - attr_reader :executable, :args, :input, :options, :processed_output, :processed_status, :path + attr_reader :executable, :args, :input, :options, :processed_output, :processed_status, :env attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed? def sudo_prefix return [] unless sudo? askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : [] - ["/usr/bin/sudo", *askpass_flags, "-E", "--"] + prefix = ["/usr/bin/sudo", *askpass_flags, "-E"] + + env.each do |name, value| + sanitized_name = Shellwords.escape(name) + sanitized_value = Shellwords.escape(value) + prefix << "#{sanitized_name}=#{sanitized_value}" + end + + prefix << "--" end def assert_success @@ -85,7 +97,7 @@ module Hbc executable, *args = expanded_command raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = - Open3.popen3({ "PATH" => path }, [executable, executable], *args, **options) + Open3.popen3(env, [executable, executable], *args, **options) write_input_to(raw_stdin) raw_stdin.close_write diff --git a/Library/Homebrew/test/cask/artifact/pkg_spec.rb b/Library/Homebrew/test/cask/artifact/pkg_spec.rb index 6d1827bdbc..da6016df4f 100644 --- a/Library/Homebrew/test/cask/artifact/pkg_spec.rb +++ b/Library/Homebrew/test/cask/artifact/pkg_spec.rb @@ -15,6 +15,11 @@ describe Hbc::Artifact::Pkg, :cask do args: ["-pkg", cask.staged_path.join("MyFancyPkg", "Fancy.pkg"), "-target", "/"], sudo: true, print_stdout: true, + env: { + "LOGNAME" => ENV["USER"], + "USER" => ENV["USER"], + "USERNAME" => ENV["USER"], + }, ) pkg.install_phase(command: fake_system_command) @@ -55,6 +60,11 @@ describe Hbc::Artifact::Pkg, :cask do args: ["-pkg", cask.staged_path.join("MyFancyPkg", "Fancy.pkg"), "-target", "/", "-applyChoiceChangesXML", cask.staged_path.join("/tmp/choices.xml")], sudo: true, print_stdout: true, + env: { + "LOGNAME" => ENV["USER"], + "USER" => ENV["USER"], + "USERNAME" => ENV["USER"], + }, ) pkg.install_phase(command: fake_system_command) diff --git a/Library/Homebrew/test/cask/system_command_spec.rb b/Library/Homebrew/test/cask/system_command_spec.rb index e9d2c4d268..4a55310788 100644 --- a/Library/Homebrew/test/cask/system_command_spec.rb +++ b/Library/Homebrew/test/cask/system_command_spec.rb @@ -1,4 +1,59 @@ describe Hbc::SystemCommand, :cask do + describe "#initialize" do + let(:env_args) { ["bash", "-c", 'printf "%s" "${A?}" "${B?}" "${C?}"'] } + + describe "given some environment variables" do + subject { + described_class.new( + "env", + args: env_args, + env: { "A" => "1", "B" => "2", "C" => "3" }, + must_succeed: true, + ) + } + + its("run!.stdout") { is_expected.to eq("123") } + + describe "the resulting command line" do + it "does not include the given variables" do + expect(Open3) + .to receive(:popen3) + .with(a_hash_including("PATH"), ["env"] * 2, *env_args, {}) + .and_call_original + + subject.run! + end + end + end + + describe "given some environment variables and sudo: true" do + subject { + described_class.new( + "env", + args: env_args, + env: { "A" => "1", "B" => "2", "C" => "3" }, + must_succeed: true, + sudo: true, + ) + } + + describe "the resulting command line" do + it "includes the given variables explicitly" do + expect(Open3) + .to receive(:popen3) + .with(an_instance_of(Hash), ["/usr/bin/sudo"] * 2, + "-E", a_string_starting_with("PATH="), + "A=1", "B=2", "C=3", "--", "env", *env_args, {}) + .and_wrap_original do |original_popen3, *_, &block| + original_popen3.call("/usr/bin/true", &block) + end + + subject.run! + end + end + end + end + describe "when the exit code is 0" do describe "its result" do subject { described_class.run("/usr/bin/true") } @@ -134,4 +189,11 @@ describe Hbc::SystemCommand, :cask do }.to be_a_success end end + + describe "given an invalid variable name" do + it "raises an ArgumentError" do + expect { described_class.run("true", env: { "1ABC" => true }) } + .to raise_error(ArgumentError, /variable name/) + end + end end diff --git a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb index b9390d4821..211b081888 100644 --- a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb +++ b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb @@ -1,5 +1,5 @@ def sudo(*args) - %w[/usr/bin/sudo -E --] + args.flatten + ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--"] + args.flatten end module Hbc diff --git a/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb b/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb index 1a010f579a..1e29456f31 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_examples/hbc_staged.rb @@ -80,7 +80,7 @@ shared_examples Hbc::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) Hbc::FakeSystemCommand.expects_command( - ["/usr/bin/sudo", "-E", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname], + ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname], ) staged.set_ownership(fake_pathname.to_s) @@ -93,7 +93,7 @@ shared_examples Hbc::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) Hbc::FakeSystemCommand.expects_command( - ["/usr/bin/sudo", "-E", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname], + ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname], ) staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s]) @@ -105,7 +105,7 @@ shared_examples Hbc::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) Hbc::FakeSystemCommand.expects_command( - ["/usr/bin/sudo", "-E", "--", "/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname], + ["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname], ) staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group")