From 19617cb16120d431d62f2195d26865f47fc886d7 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Wed, 17 Jul 2024 05:26:43 +0100 Subject: [PATCH 1/2] system_command: add reset_uid option --- Library/Homebrew/sorbet/rbi/parlour.rbi | 3 + Library/Homebrew/system_command.rb | 66 ++++++++++++++++---- Library/Homebrew/test/system_command_spec.rb | 26 ++++---- 3 files changed, 71 insertions(+), 24 deletions(-) diff --git a/Library/Homebrew/sorbet/rbi/parlour.rbi b/Library/Homebrew/sorbet/rbi/parlour.rbi index ec4254527f..78ebc316c5 100644 --- a/Library/Homebrew/sorbet/rbi/parlour.rbi +++ b/Library/Homebrew/sorbet/rbi/parlour.rbi @@ -279,6 +279,9 @@ class SystemCommand sig { returns(T::Boolean) } def must_succeed?; end + + sig { returns(T::Boolean) } + def reset_uid?; end end module Utils diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index 31915d1f5a..94ae4444ce 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -2,7 +2,6 @@ # frozen_string_literal: true require "attrable" -require "open3" require "plist" require "shellwords" require "uri" @@ -92,6 +91,7 @@ class SystemCommand verbose: T.nilable(T::Boolean), secrets: T.any(String, T::Array[String]), chdir: T.any(String, Pathname), + reset_uid: T::Boolean, timeout: T.nilable(T.any(Integer, Float)), ).void } @@ -109,6 +109,7 @@ class SystemCommand verbose: false, secrets: [], chdir: T.unsafe(nil), + reset_uid: false, timeout: nil ) require "extend/ENV" @@ -140,6 +141,7 @@ class SystemCommand @verbose = verbose @secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq @chdir = chdir + @reset_uid = reset_uid @timeout = timeout end @@ -152,7 +154,7 @@ class SystemCommand attr_reader :executable, :args, :input, :chdir, :env - attr_predicate :sudo?, :sudo_as_root?, :must_succeed? + attr_predicate :sudo?, :sudo_as_root?, :must_succeed?, :reset_uid? sig { returns(T::Boolean) } def debug? @@ -238,12 +240,7 @@ class SystemCommand } options[:chdir] = chdir if chdir - raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = Open3.popen3( - env.merge({ "COLUMNS" => Tty.width.to_s }), - [executable, executable], - *args, - **options, - ) + raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = exec3(env, executable, *args, **options) write_input_to(raw_stdin) raw_stdin.close_write @@ -276,9 +273,56 @@ class SystemCommand rescue Interrupt Process.kill("INT", raw_wait_thr.pid) if raw_wait_thr && !sudo? raise Interrupt - rescue SystemCallError => e - @status = $CHILD_STATUS - @output << [:stderr, e.message] + ensure + raw_stdin&.close + raw_stdout&.close + raw_stderr&.close + end + + sig { + params( + env: T::Hash[String, String], + executable: String, + args: String, + options: T.untyped, + ).returns([IO, IO, IO, Thread]) + } + def exec3(env, executable, *args, **options) + in_r, in_w = IO.pipe + options[:in] = in_r + in_w.sync = true + + out_r, out_w = IO.pipe + options[:out] = out_w + + err_r, err_w = IO.pipe + options[:err] = err_w + + pid = fork do + Process::UID.change_privilege(Process.euid) if reset_uid? && Process.euid != Process.uid + + exec( + env.merge({ "COLUMNS" => Tty.width.to_s }), + [executable, executable], + *args, + **options, + ) + rescue SystemCallError => e + $stderr.puts(e.message) + exit!(127) + end + wait_thr = Process.detach(pid) + + [in_w, out_r, err_r, wait_thr] + rescue + in_w&.close + out_r&.close + err_r&.close + raise + ensure + in_r&.close + out_w&.close + err_w&.close end sig { params(raw_stdin: IO).void } diff --git a/Library/Homebrew/test/system_command_spec.rb b/Library/Homebrew/test/system_command_spec.rb index d09612a1fb..210dd142c8 100644 --- a/Library/Homebrew/test/system_command_spec.rb +++ b/Library/Homebrew/test/system_command_spec.rb @@ -25,10 +25,10 @@ RSpec.describe SystemCommand do describe "the resulting command line" do it "includes the given variables explicitly" do - expect(Open3) - .to receive(:popen3) + expect(command) + .to receive(:exec3) .with( - an_instance_of(Hash), ["/usr/bin/env", "/usr/bin/env"], "A=1", "B=2", "C=3", + an_instance_of(Hash), "/usr/bin/env", "A=1", "B=2", "C=3", "env", *env_args, pgroup: true ) @@ -55,14 +55,14 @@ RSpec.describe SystemCommand do describe "the resulting command line" do it "includes the given variables explicitly" do - expect(Open3) - .to receive(:popen3) + expect(command) + .to receive(:exec3) .with( - an_instance_of(Hash), ["/usr/bin/sudo", "/usr/bin/sudo"], "-E", + an_instance_of(Hash), "/usr/bin/sudo", "-E", "A=1", "B=2", "C=3", "--", "env", *env_args, pgroup: nil ) - .and_wrap_original do |original_popen3, *_, &block| - original_popen3.call("true", &block) + .and_wrap_original do |original_exec3, *_, &block| + original_exec3.call({}, "true", &block) end command.run! @@ -76,14 +76,14 @@ RSpec.describe SystemCommand do describe "the resulting command line" do it "includes the given variables explicitly" do - expect(Open3) - .to receive(:popen3) + expect(command) + .to receive(:exec3) .with( - an_instance_of(Hash), ["/usr/bin/sudo", "/usr/bin/sudo"], "-u", "root", + an_instance_of(Hash), "/usr/bin/sudo", "-u", "root", "-E", "A=1", "B=2", "C=3", "--", "env", *env_args, pgroup: nil ) - .and_wrap_original do |original_popen3, *_, &block| - original_popen3.call("true", &block) + .and_wrap_original do |original_exec3, *_, &block| + original_exec3.call({}, "true", &block) end command.run! From ec41f66ee7771cf74f840ce60239ac968e55a535 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Wed, 17 Jul 2024 05:27:05 +0100 Subject: [PATCH 2/2] cask/artifact/installer: reset UID when running scripts --- Library/Homebrew/cask/artifact/installer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/artifact/installer.rb b/Library/Homebrew/cask/artifact/installer.rb index b8ff7e405a..76b18abb3d 100644 --- a/Library/Homebrew/cask/artifact/installer.rb +++ b/Library/Homebrew/cask/artifact/installer.rb @@ -35,9 +35,10 @@ module Cask command.run!( executable_path, **args, - env: { "PATH" => PATH.new( + env: { "PATH" => PATH.new( HOMEBREW_PREFIX/"bin", HOMEBREW_PREFIX/"sbin", ENV.fetch("PATH") ) }, + reset_uid: true, ) end end