From d00f35b8c4dcdcbf86074015f0471f27e3dc2aaa Mon Sep 17 00:00:00 2001 From: Yoshimasa Niwa Date: Sun, 9 Oct 2016 13:27:31 -0700 Subject: [PATCH 1/4] FIX: cask changes /usr/local ownership recursively Relatively old code in Hbc::Caskroom recursively changes the ownership of the directory where the Caskroom directory exists, that changes entire files in /usr/local to user:staff if Homebrew setup with default configuration. This is really dangerous because it's easy to trigger (just simply type `brew cask something` by following some installation documentation.) This patch removes entire `chown` with -R option and make the logic simply creating Caskroom directory with default Homebrew directories ownership and permission. --- Library/Homebrew/cask/lib/hbc/caskroom.rb | 31 +++++++++-------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/caskroom.rb b/Library/Homebrew/cask/lib/hbc/caskroom.rb index 583cac34a0..6375345ee1 100644 --- a/Library/Homebrew/cask/lib/hbc/caskroom.rb +++ b/Library/Homebrew/cask/lib/hbc/caskroom.rb @@ -13,7 +13,7 @@ module Hbc FileUtils.mv repo_caskroom, Hbc.caskroom else opoo "#{Hbc.caskroom.parent} is not writable, sudo is needed to move the Caskroom." - system "/usr/bin/sudo", "--", "/bin/mv", "--", repo_caskroom.to_s, Hbc.caskroom.parent.to_s + sudo "/bin/mv", repo_caskroom.to_s, Hbc.caskroom.parent.to_s end end @@ -21,24 +21,17 @@ module Hbc return if Hbc.caskroom.exist? ohai "Creating Caskroom at #{Hbc.caskroom}" - if Hbc.caskroom.parent.writable? - Hbc.caskroom.mkpath - else - ohai "We'll set permissions properly so we won't need sudo in the future" - toplevel_dir = Hbc.caskroom - toplevel_dir = toplevel_dir.parent until toplevel_dir.parent.root? - unless toplevel_dir.directory? - # If a toplevel dir such as '/opt' must be created, enforce standard permissions. - # sudo in system is rude. - system "/usr/bin/sudo", "--", "/bin/mkdir", "--", toplevel_dir - system "/usr/bin/sudo", "--", "/bin/chmod", "--", "0775", toplevel_dir - end - # sudo in system is rude. - system "/usr/bin/sudo", "--", "/bin/mkdir", "-p", "--", Hbc.caskroom - unless Hbc.caskroom.parent == toplevel_dir - system "/usr/bin/sudo", "--", "/usr/sbin/chown", "-R", "--", "#{Utils.current_user}:staff", Hbc.caskroom.parent.to_s - end - end + ohai "We'll set permissions properly so we won't need sudo in the future" + + sudo "/bin/mkdir", "-p", Hbc.caskroom + sudo "/bin/chmod", "g+rwx", Hbc.caskroom + sudo "/usr/sbin/chown", Utils.current_user, Hbc.caskroom + sudo "/usr/bin/chgrp", "admin", Hbc.caskroom + end + + def sudo(*args) + ohai "/usr/bin/sudo #{args.join(" ")}" + system "/usr/bin/sudo", *args end end end From d51cd15e0c748d1b2a35f6327a131622b6ee48b7 Mon Sep 17 00:00:00 2001 From: Yoshimasa Niwa Date: Mon, 10 Oct 2016 11:11:52 -0700 Subject: [PATCH 2/4] Create caskroom without sudo in writable parent. In case the parent directory of Caskroom is writable for the user, we don't need to use `sudo` to execute commands. Make a generic method to run commands that has an option to switch sudo so that we can run commands with and without sudo. --- Library/Homebrew/cask/lib/hbc/caskroom.rb | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/caskroom.rb b/Library/Homebrew/cask/lib/hbc/caskroom.rb index 6375345ee1..a8af580011 100644 --- a/Library/Homebrew/cask/lib/hbc/caskroom.rb +++ b/Library/Homebrew/cask/lib/hbc/caskroom.rb @@ -13,7 +13,7 @@ module Hbc FileUtils.mv repo_caskroom, Hbc.caskroom else opoo "#{Hbc.caskroom.parent} is not writable, sudo is needed to move the Caskroom." - sudo "/bin/mv", repo_caskroom.to_s, Hbc.caskroom.parent.to_s + command "/bin/mv", repo_caskroom, Hbc.caskroom.parent, :use_sudo => true end end @@ -22,16 +22,23 @@ module Hbc ohai "Creating Caskroom at #{Hbc.caskroom}" ohai "We'll set permissions properly so we won't need sudo in the future" + use_sudo = !Hbc.caskroom.parent.writable? - sudo "/bin/mkdir", "-p", Hbc.caskroom - sudo "/bin/chmod", "g+rwx", Hbc.caskroom - sudo "/usr/sbin/chown", Utils.current_user, Hbc.caskroom - sudo "/usr/bin/chgrp", "admin", Hbc.caskroom + command "/bin/mkdir", "-p", Hbc.caskroom, :use_sudo => use_sudo + command "/bin/chmod", "g+rwx", Hbc.caskroom, :use_sudo => use_sudo + command "/usr/sbin/chown", Utils.current_user, Hbc.caskroom, :use_sudo => use_sudo + command "/usr/bin/chgrp", "admin", Hbc.caskroom, :use_sudo => use_sudo end - def sudo(*args) - ohai "/usr/bin/sudo #{args.join(" ")}" - system "/usr/bin/sudo", *args + def command(*args) + options = args.last.is_a?(Hash) ? args.pop : {} + + if options[:use_sudo] + args.unshift "/usr/bin/sudo" + end + + ohai args.join(" ") + system *args end end end From 5d606dd0a275446f00f31884ac042572f3a620ad Mon Sep 17 00:00:00 2001 From: Yoshimasa Niwa Date: Sat, 15 Oct 2016 15:18:51 -0700 Subject: [PATCH 3/4] Use SystemCommand.run instead of custom wrapper. There is an existing `SystemCommand.run` that executes command with `sudo`. Use that instead of yet another custom wrapper. --- Library/Homebrew/cask/lib/hbc/caskroom.rb | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/caskroom.rb b/Library/Homebrew/cask/lib/hbc/caskroom.rb index a8af580011..6370fed3fa 100644 --- a/Library/Homebrew/cask/lib/hbc/caskroom.rb +++ b/Library/Homebrew/cask/lib/hbc/caskroom.rb @@ -13,7 +13,7 @@ module Hbc FileUtils.mv repo_caskroom, Hbc.caskroom else opoo "#{Hbc.caskroom.parent} is not writable, sudo is needed to move the Caskroom." - command "/bin/mv", repo_caskroom, Hbc.caskroom.parent, :use_sudo => true + SystemCommand.run("/bin/mv", :args => [repo_caskroom, Hbc.caskroom.parent], :sudo => true) end end @@ -22,23 +22,12 @@ module Hbc ohai "Creating Caskroom at #{Hbc.caskroom}" ohai "We'll set permissions properly so we won't need sudo in the future" - use_sudo = !Hbc.caskroom.parent.writable? + sudo = !Hbc.caskroom.parent.writable? - command "/bin/mkdir", "-p", Hbc.caskroom, :use_sudo => use_sudo - command "/bin/chmod", "g+rwx", Hbc.caskroom, :use_sudo => use_sudo - command "/usr/sbin/chown", Utils.current_user, Hbc.caskroom, :use_sudo => use_sudo - command "/usr/bin/chgrp", "admin", Hbc.caskroom, :use_sudo => use_sudo - end - - def command(*args) - options = args.last.is_a?(Hash) ? args.pop : {} - - if options[:use_sudo] - args.unshift "/usr/bin/sudo" - end - - ohai args.join(" ") - system *args + SystemCommand.run("/bin/mkdir", :args => ["-p", Hbc.caskroom], :sudo => sudo) + SystemCommand.run("/bin/chmod", :args => ["g+rwx", Hbc.caskroom], :sudo => sudo) + SystemCommand.run("/usr/sbin/chown", :args => [Utils.current_user, Hbc.caskroom], :sudo => sudo) + SystemCommand.run("/usr/bin/chgrp", :args => ["admin", Hbc.caskroom], :sudo => sudo) end end end From 60c4328a1795c879f96488618de1eec1521ec3ef Mon Sep 17 00:00:00 2001 From: Yoshimasa Niwa Date: Wed, 19 Oct 2016 11:14:30 -0700 Subject: [PATCH 4/4] Use Ruby 2.x style for named arguments. --- Library/Homebrew/cask/lib/hbc/caskroom.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/caskroom.rb b/Library/Homebrew/cask/lib/hbc/caskroom.rb index 6370fed3fa..7bc8294e15 100644 --- a/Library/Homebrew/cask/lib/hbc/caskroom.rb +++ b/Library/Homebrew/cask/lib/hbc/caskroom.rb @@ -13,7 +13,7 @@ module Hbc FileUtils.mv repo_caskroom, Hbc.caskroom else opoo "#{Hbc.caskroom.parent} is not writable, sudo is needed to move the Caskroom." - SystemCommand.run("/bin/mv", :args => [repo_caskroom, Hbc.caskroom.parent], :sudo => true) + SystemCommand.run("/bin/mv", args: [repo_caskroom, Hbc.caskroom.parent], sudo: true) end end @@ -24,10 +24,10 @@ module Hbc ohai "We'll set permissions properly so we won't need sudo in the future" sudo = !Hbc.caskroom.parent.writable? - SystemCommand.run("/bin/mkdir", :args => ["-p", Hbc.caskroom], :sudo => sudo) - SystemCommand.run("/bin/chmod", :args => ["g+rwx", Hbc.caskroom], :sudo => sudo) - SystemCommand.run("/usr/sbin/chown", :args => [Utils.current_user, Hbc.caskroom], :sudo => sudo) - SystemCommand.run("/usr/bin/chgrp", :args => ["admin", Hbc.caskroom], :sudo => sudo) + SystemCommand.run("/bin/mkdir", args: ["-p", Hbc.caskroom], sudo: sudo) + SystemCommand.run("/bin/chmod", args: ["g+rwx", Hbc.caskroom], sudo: sudo) + SystemCommand.run("/usr/sbin/chown", args: [Utils.current_user, Hbc.caskroom], sudo: sudo) + SystemCommand.run("/usr/bin/chgrp", args: ["admin", Hbc.caskroom], sudo: sudo) end end end