Merge pull request #14629 from Kentzo/sudo-user

sudo: explicitly specify the root user where necessary

Library/Homebrew/cask/artifact/abstract_uninstall.rb

@@ -105,30 +105,47 @@ module Cask
 
         all_services.each do |service|
           ohai "Removing launchctl service #{service}"
-          booleans.each do |with_sudo|
+          booleans.each do |sudo|
             plist_status = command.run(
               "/bin/launchctl",
-              args: ["list", service],
-              sudo: with_sudo, print_stderr: false
+              args:         ["list", service],
+              sudo:         sudo,
+              sudo_as_root: sudo,
+              print_stderr: false,
             ).stdout
             if plist_status.start_with?("{")
-              command.run!("/bin/launchctl", args: ["remove", service], sudo: with_sudo)
+              command.run!(
+                "/bin/launchctl",
+                args:         ["remove", service],
+                sudo:         sudo,
+                sudo_as_root: sudo,
+              )
               sleep 1
             end
             paths = [
               +"/Library/LaunchAgents/#{service}.plist",
               +"/Library/LaunchDaemons/#{service}.plist",
             ]
-            paths.each { |elt| elt.prepend(Dir.home).freeze } unless with_sudo
+            paths.each { |elt| elt.prepend(Dir.home).freeze } unless sudo
             paths = paths.map { |elt| Pathname(elt) }.select(&:exist?)
             paths.each do |path|
-              command.run!("/bin/rm", args: ["-f", "--", path], sudo: with_sudo)
+              command.run!("/bin/rm", args: ["-f", "--", path], sudo: sudo, sudo_as_root: sudo)
             end
             # undocumented and untested: pass a path to uninstall :launchctl
             next unless Pathname(service).exist?
 
-            command.run!("/bin/launchctl", args: ["unload", "-w", "--", service], sudo: with_sudo)
-            command.run!("/bin/rm", args: ["-f", "--", service], sudo: with_sudo)
+            command.run!(
+              "/bin/launchctl",
+              args:         ["unload", "-w", "--", service],
+              sudo:         sudo,
+              sudo_as_root: sudo,
+            )
+            command.run!(
+              "/bin/rm",
+              args:         ["-f", "--", service],
+              sudo:         sudo,
+              sudo_as_root: sudo,
+            )
             sleep 1
           end
         end
@@ -301,14 +318,35 @@ module Cask
       def uninstall_kext(*kexts, command: nil, **_)
         kexts.each do |kext|
           ohai "Unloading kernel extension #{kext}"
-          is_loaded = system_command!("/usr/sbin/kextstat", args: ["-l", "-b", kext], sudo: true).stdout
+          is_loaded = system_command!(
+            "/usr/sbin/kextstat",
+            args:         ["-l", "-b", kext],
+            sudo:         true,
+            sudo_as_root: true,
+          ).stdout
           if is_loaded.length > 1
-            system_command!("/sbin/kextunload", args: ["-b", kext], sudo: true)
+            system_command!(
+              "/sbin/kextunload",
+              args:         ["-b", kext],
+              sudo:         true,
+              sudo_as_root: true,
+            )
             sleep 1
           end
-          system_command!("/usr/sbin/kextfind", args: ["-b", kext], sudo: true).stdout.chomp.lines.each do |kext_path|
+          found_kexts = system_command!(
+            "/usr/sbin/kextfind",
+            args:         ["-b", kext],
+            sudo:         true,
+            sudo_as_root: true,
+          ).stdout.chomp.lines
+          found_kexts.each do |kext_path|
             ohai "Removing kernel extension #{kext_path}"
-            system_command!("/bin/rm", args: ["-rf", kext_path], sudo: true)
+            system_command!(
+              "/bin/rm",
+              args:         ["-rf", kext_path],
+              sudo:         true,
+              sudo_as_root: true,
+            )
           end
         end
       end

Library/Homebrew/cask/artifact/keyboard_layout.rb

@@ -22,7 +22,12 @@ module Cask
       private
 
       def delete_keyboard_layout_cache(command: nil, **_)
-        command.run!("/bin/rm", args: ["-f", "--", "/System/Library/Caches/com.apple.IntlDataCache.le*"], sudo: true)
+        command.run!(
+          "/bin/rm",
+          args:         ["-f", "--", "/System/Library/Caches/com.apple.IntlDataCache.le*"],
+          sudo:         true,
+          sudo_as_root: true,
+        )
       end
     end
   end

Library/Homebrew/cask/artifact/pkg.rb

@@ -62,7 +62,14 @@ module Cask
             "USER"     => User.current,
             "USERNAME" => User.current,
           }
-          command.run!("/usr/sbin/installer", sudo: true, args: args, print_stdout: true, env: env)
+          command.run!(
+            "/usr/sbin/installer",
+            sudo:         true,
+            sudo_as_root: true,
+            args:         args,
+            print_stdout: true,
+            env:          env,
+          )
         end
       end
 

Library/Homebrew/cask/pkg.rb

@@ -30,9 +30,10 @@ module Cask
         odebug "Deleting pkg files"
         @command.run!(
           "/usr/bin/xargs",
-          args:  ["-0", "--", "/bin/rm", "--"],
-          input: pkgutil_bom_files.join("\0"),
-          sudo:  true,
+          args:         ["-0", "--", "/bin/rm", "--"],
+          input:        pkgutil_bom_files.join("\0"),
+          sudo:         true,
+          sudo_as_root: true,
         )
       end
 
@@ -40,9 +41,10 @@ module Cask
         odebug "Deleting pkg symlinks and special files"
         @command.run!(
           "/usr/bin/xargs",
-          args:  ["-0", "--", "/bin/rm", "--"],
-          input: pkgutil_bom_specials.join("\0"),
-          sudo:  true,
+          args:         ["-0", "--", "/bin/rm", "--"],
+          input:        pkgutil_bom_specials.join("\0"),
+          sudo:         true,
+          sudo_as_root: true,
         )
       end
 
@@ -59,7 +61,12 @@ module Cask
     sig { void }
     def forget
       odebug "Unregistering pkg receipt (aka forgetting)"
-      @command.run!("/usr/sbin/pkgutil", args: ["--forget", package_id], sudo: true)
+      @command.run!(
+        "/usr/sbin/pkgutil",
+        args:         ["--forget", package_id],
+        sudo:         true,
+        sudo_as_root: true,
+      )
     end
 
     sig { returns(T::Array[Pathname]) }
@@ -112,9 +119,10 @@ module Cask
     def rmdir(path)
       @command.run!(
         "/usr/bin/xargs",
-        args:  ["-0", "--", RMDIR_SH.to_s],
-        input: Array(path).join("\0"),
-        sudo:  true,
+        args:         ["-0", "--", RMDIR_SH.to_s],
+        input:        Array(path).join("\0"),
+        sudo:         true,
+        sudo_as_root: true,
       )
     end
 

Library/Homebrew/sorbet/rbi/parlour.rbi

@@ -107,6 +107,9 @@ class SystemCommand
   sig { returns(T::Boolean) }
   def sudo?; end
 
+  sig { returns(T::Boolean) }
+  def sudo_as_root?; end
+
   sig { returns(T::Boolean) }
   def print_stdout?; end
 

Library/Homebrew/system_command.rb

@@ -65,6 +65,7 @@ class SystemCommand
       executable:   T.any(String, Pathname),
       args:         T::Array[T.any(String, Integer, Float, URI::Generic)],
       sudo:         T::Boolean,
+      sudo_as_root: T::Boolean,
       env:          T::Hash[String, String],
       input:        T.any(String, T::Array[String]),
       must_succeed: T::Boolean,
@@ -81,6 +82,7 @@ class SystemCommand
     executable,
     args: [],
     sudo: false,
+    sudo_as_root: false,
     env: {},
     input: [],
     must_succeed: false,
@@ -95,7 +97,11 @@ class SystemCommand
     require "extend/ENV"
     @executable = executable
     @args = args
+
+    raise ArgumentError, "sudo_as_root cannot be set if sudo is false" if !sudo && sudo_as_root
+
     @sudo = sudo
+    @sudo_as_root = sudo_as_root
     env.each_key do |name|
       next if /^[\w&&\D]\w*$/.match?(name)
 
@@ -122,7 +128,7 @@ class SystemCommand
 
   attr_reader :executable, :args, :input, :chdir, :env
 
-  attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed?
+  attr_predicate :sudo?, :sudo_as_root?, :print_stdout?, :print_stderr?, :must_succeed?
 
   sig { returns(T::Boolean) }
   def debug?
@@ -153,8 +159,10 @@ class SystemCommand
 
   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"] : []
-    ["/usr/bin/sudo", *askpass_flags, "-E", *env_args, "--"]
+    ["/usr/bin/sudo", *user_flags, *askpass_flags, "-E", *env_args, "--"]
   end
 
   sig { returns(T::Array[String]) }

Library/Homebrew/test/cask/artifact/pkg_spec.rb

@@ -16,6 +16,7 @@ describe Cask::Artifact::Pkg, :cask do
         "/usr/sbin/installer",
         args:         ["-pkg", cask.staged_path.join("MyFancyPkg", "Fancy.pkg"), "-target", "/"],
         sudo:         true,
+        sudo_as_root: true,
         print_stdout: true,
         env:          {
           "LOGNAME"  => ENV.fetch("USER"),
@@ -65,6 +66,7 @@ describe Cask::Artifact::Pkg, :cask do
           cask.staged_path.join("/tmp/choices.xml")
         ],
         sudo:         true,
+        sudo_as_root: true,
         print_stdout: true,
         env:          {
           "LOGNAME"  => ENV.fetch("USER"),

Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb

@@ -31,14 +31,26 @@ shared_examples "#uninstall_phase or #zap_phase" do
 
     it "works when job is owned by user" do
       allow(fake_system_command).to receive(:run)
-        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: false)
+        .with(
+          "/bin/launchctl",
+          args:         ["list", "my.fancy.package.service"],
+          print_stderr: false,
+          sudo:         false,
+          sudo_as_root: false,
+        )
         .and_return(instance_double(SystemCommand::Result, stdout: service_info))
       allow(fake_system_command).to receive(:run)
-        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: true)
+        .with(
+          "/bin/launchctl",
+          args:         ["list", "my.fancy.package.service"],
+          print_stderr: false,
+          sudo:         true,
+          sudo_as_root: true,
+        )
         .and_return(instance_double(SystemCommand::Result, stdout: unknown_response))
 
       expect(fake_system_command).to receive(:run!)
-        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: false)
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: false, sudo_as_root: false)
         .and_return(instance_double(SystemCommand::Result))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
@@ -46,14 +58,26 @@ shared_examples "#uninstall_phase or #zap_phase" do
 
     it "works when job is owned by system" do
       allow(fake_system_command).to receive(:run)
-        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: false)
+        .with(
+          "/bin/launchctl",
+          args:         ["list", "my.fancy.package.service"],
+          print_stderr: false,
+          sudo:         false,
+          sudo_as_root: false,
+        )
         .and_return(instance_double(SystemCommand::Result, stdout: unknown_response))
       allow(fake_system_command).to receive(:run)
-        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: true)
+        .with(
+          "/bin/launchctl",
+          args:         ["list", "my.fancy.package.service"],
+          print_stderr: false,
+          sudo:         true,
+          sudo_as_root: true,
+        )
         .and_return(instance_double(SystemCommand::Result, stdout: service_info))
 
       expect(fake_system_command).to receive(:run!)
-        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: true)
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: true, sudo_as_root: true)
         .and_return(instance_double(SystemCommand::Result))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
@@ -94,14 +118,26 @@ shared_examples "#uninstall_phase or #zap_phase" do
         .and_return(["my.fancy.package.service.12345"])
 
       allow(fake_system_command).to receive(:run)
-        .with("/bin/launchctl", args: ["list", "my.fancy.package.service.12345"], print_stderr: false, sudo: false)
+        .with(
+          "/bin/launchctl",
+          args:         ["list", "my.fancy.package.service.12345"],
+          print_stderr: false,
+          sudo:         false,
+          sudo_as_root: false,
+        )
         .and_return(instance_double(SystemCommand::Result, stdout: unknown_response))
       allow(fake_system_command).to receive(:run)
-        .with("/bin/launchctl", args: ["list", "my.fancy.package.service.12345"], print_stderr: false, sudo: true)
+        .with(
+          "/bin/launchctl",
+          args:         ["list", "my.fancy.package.service.12345"],
+          print_stderr: false,
+          sudo:         true,
+          sudo_as_root: true,
+        )
         .and_return(instance_double(SystemCommand::Result, stdout: service_info))
 
       expect(fake_system_command).to receive(:run!)
-        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service.12345"], sudo: true)
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service.12345"], sudo: true, sudo_as_root: true)
         .and_return(instance_double(SystemCommand::Result))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
@@ -148,19 +184,19 @@ shared_examples "#uninstall_phase or #zap_phase" do
 
     it "is supported" do
       allow(subject).to receive(:system_command!)
-        .with("/usr/sbin/kextstat", args: ["-l", "-b", kext_id], sudo: true)
+        .with("/usr/sbin/kextstat", args: ["-l", "-b", kext_id], sudo: true, sudo_as_root: true)
         .and_return(instance_double("SystemCommand::Result", stdout: "loaded"))
 
       expect(subject).to receive(:system_command!)
-        .with("/sbin/kextunload", args: ["-b", kext_id], sudo: true)
+        .with("/sbin/kextunload", args: ["-b", kext_id], sudo: true, sudo_as_root: true)
         .and_return(instance_double("SystemCommand::Result"))
 
       expect(subject).to receive(:system_command!)
-        .with("/usr/sbin/kextfind", args: ["-b", kext_id], sudo: true)
+        .with("/usr/sbin/kextfind", args: ["-b", kext_id], sudo: true, sudo_as_root: true)
         .and_return(instance_double("SystemCommand::Result", stdout: "/Library/Extensions/FancyPackage.kext\n"))
 
       expect(subject).to receive(:system_command!)
-        .with("/bin/rm", args: ["-rf", "/Library/Extensions/FancyPackage.kext"], sudo: true)
+        .with("/bin/rm", args: ["-rf", "/Library/Extensions/FancyPackage.kext"], sudo: true, sudo_as_root: true)
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
     end
@@ -281,13 +317,14 @@ shared_examples "#uninstall_phase or #zap_phase" do
       it "is supported" do
         allow(fake_system_command).to receive(:run).with(any_args).and_call_original
 
-        expect(fake_system_command).to receive(:run).with(
-          cask.staged_path.join("MyFancyPkg", "FancyUninstaller.tool"),
-          args:         ["--please"],
-          must_succeed: true,
-          print_stdout: true,
-          sudo:         false,
-        )
+        expect(fake_system_command).to receive(:run)
+          .with(
+            cask.staged_path.join("MyFancyPkg", "FancyUninstaller.tool"),
+            args:         ["--please"],
+            must_succeed: true,
+            print_stdout: true,
+            sudo:         false,
+          )
 
         InstallHelper.install_without_artifacts(cask)
         subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)

Library/Homebrew/test/cask/dsl/shared_examples/staged.rb

@@ -67,7 +67,11 @@ shared_examples Cask::Staged do
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
     expect(fake_system_command).to receive(:run!)
-      .with("/usr/sbin/chown", args: ["-R", "--", "fake_user:staff", fake_pathname, fake_pathname], sudo: true)
+      .with(
+        "/usr/sbin/chown",
+        args: ["-R", "--", "fake_user:staff", fake_pathname, fake_pathname],
+        sudo: true,
+      )
 
     staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s])
   end
@@ -78,7 +82,11 @@ shared_examples Cask::Staged do
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
     expect(fake_system_command).to receive(:run!)
-      .with("/usr/sbin/chown", args: ["-R", "--", "other_user:other_group", fake_pathname], sudo: true)
+      .with(
+        "/usr/sbin/chown",
+        args: ["-R", "--", "other_user:other_group", fake_pathname],
+        sudo: true,
+      )
 
     staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group")
   end

Library/Homebrew/test/cask/pkg_spec.rb

@@ -63,8 +63,9 @@ describe Cask::Pkg, :cask do
 
         expect(fake_system_command).to receive(:run!).with(
           "/usr/sbin/pkgutil",
-          args: ["--forget", "my.fake.pkg"],
-          sudo: true,
+          args:         ["--forget", "my.fake.pkg"],
+          sudo:         true,
+          sudo_as_root: true,
         )
 
         pkg.uninstall
@@ -114,9 +115,10 @@ describe Cask::Pkg, :cask do
       allow(fake_system_command).to receive(:run!).and_call_original
       expect(fake_system_command).to receive(:run!).with(
         "/usr/bin/xargs",
-        args:  ["-0", "--", a_string_including("rmdir")],
-        input: [fake_dir].join("\0"),
-        sudo:  true,
+        args:         ["-0", "--", a_string_including("rmdir")],
+        input:        [fake_dir].join("\0"),
+        sudo:         true,
+        sudo_as_root: true,
       ).and_return(instance_double(SystemCommand::Result, stdout: ""))
 
       pkg.uninstall

Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-script-app.rb

@@ -15,7 +15,8 @@ cask "with-uninstall-script-app" do
   end
 
   uninstall script: {
-    executable: "#{appdir}/MyFancyApp.app/uninstall.sh",
-    sudo:       false,
+    executable:   "#{appdir}/MyFancyApp.app/uninstall.sh",
+    sudo:         false,
+    sudo_as_root: false,
   }
 end

Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-script-user-relative.rb

@@ -15,7 +15,8 @@ cask "with-uninstall-script-user-relative" do
   end
 
   uninstall script: {
-    executable: "~/MyFancyApp.app/uninstall.sh",
-    sudo:       false,
+    executable:   "~/MyFancyApp.app/uninstall.sh",
+    sudo:         false,
+    sudo_as_root: false,
   }
 end

Library/Homebrew/test/support/helper/cask/never_sudo_system_command.rb

@@ -5,6 +5,6 @@ require "system_command"
 
 class NeverSudoSystemCommand < SystemCommand
   def self.run(command, **options)
-    super(command, **options.merge(sudo: false))
+    super(command, **options.merge(sudo: false, sudo_as_root: false))
   end
 end

Library/Homebrew/test/system_command_spec.rb

@@ -9,12 +9,14 @@ describe SystemCommand do
         env:          env,
         must_succeed: true,
         sudo:         sudo,
+        sudo_as_root: sudo_as_root,
       )
     end
 
     let(:env_args) { ["bash", "-c", 'printf "%s" "${A?}" "${B?}" "${C?}"'] }
     let(:env) { { "A" => "1", "B" => "2", "C" => "3" } }
     let(:sudo) { false }
+    let(:sudo_as_root) { false }
 
     context "when given some environment variables" do
       its("run!.stdout") { is_expected.to eq("123") }
@@ -45,8 +47,9 @@ describe SystemCommand do
       end
     end
 
-    context "when given some environment variables and sudo: true" do
+    context "when given some environment variables and sudo: true, sudo_as_root: false" do
       let(:sudo) { true }
+      let(:sudo_as_root) { false }
 
       describe "the resulting command line" do
         it "includes the given variables explicitly" do
@@ -64,6 +67,27 @@ describe SystemCommand do
         end
       end
     end
+
+    context "when given some environment variables and sudo: true, sudo_as_root: true" do
+      let(:sudo) { true }
+      let(:sudo_as_root) { 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", "/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)
+            end
+
+          command.run!
+        end
+      end
+    end
   end
 
   context "when the exit code is 0" do