Latest brew install needs extended sudo rights #109

Sauraus · 2016-10-05T02:35:35Z

This fixes: #105

kameghamegha · 2016-11-07T18:27:04Z

Hey, @Sauraus, have you completed the DCO step? https://blog.chef.io/2016/09/19/introducing-developer-certificate-of-origin/ Here is some more info.

bdangit · 2016-11-18T15:13:31Z

would really love this fix right now. 👍

ed-flanagan · 2016-11-29T00:48:42Z

recipes/default.rb

-  user homebrew_owner
-  not_if { ::File.exist? '/usr/local/bin/brew' }
+begin
+  template '/etc/sudoers.d/homebrew_sudo' do


Fails if /etc/sudoers.d doesn't exist.

Something like...

... directory '/etc/sudoers.d' do action :nothing end template '/etc/sudoers.d/homebrew_sudo' do notifies :create, 'directory[/etc/sudoers.d]', :before ...

...might help/fix it

/etc/sudoers.d can only be missing if someone messed around with the system, not sure we need to account for people's tinkering with system paths.

Tried running the patch on a fresh install of Mavericks.
/etc/sudoers.d didn't seem to exist beforehand.

Other /etc directories like /etc/ssh don't exist either.

Mavericks... isn't that a little out dated and thus insecure to be used for anything?

But I do see your point, I've only done testing on 'new' OS X versions.

I'm with you that it's older. However, chef-dk still supports 10.9. Plus the metadata.rb file doesn't have any macOS version restrictions currently.

discentem · 2016-12-24T20:23:35Z

Why has this not been merged?

andrew-cc · 2017-01-10T19:04:28Z

@Sauraus There are a bunch of failures on CI from Rubocop—perhaps getting those fixed will move this along to getting merged 🙂

andrew-cc · 2017-01-10T20:29:26Z

Additional errors I’m getting with this change applied onto 3.0.0:

    ==> Searching online for the Command Line Tools
    ==> /usr/bin/sudo /usr/bin/touch /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    ==> Installing Command Line Tools (macOS Sierra version 10.12) for Xcode-8.2
    ==> /usr/bin/sudo /usr/sbin/softwareupdate -i Command\ Line\ Tools\ (macOS\ Sierra\ version\ 10.12)\ for\ Xcode-8.2
    Software Update Tool
    Copyright 2002-2015 Apple Inc.


    Downloading Command Line Tools (macOS Sierra version 10.12) for Xcode
    Downloaded Command Line Tools (macOS Sierra version 10.12) for Xcode
    Installing Command Line Tools (macOS Sierra version 10.12) for Xcode
    Done with Command Line Tools (macOS Sierra version 10.12) for Xcode
    Done.
    ==> /usr/bin/sudo /bin/rm -f /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    ==> /usr/bin/sudo /usr/bin/xcode-select --switch /Library/Developer/CommandLineTools
    STDERR: sudo: no tty present and no askpass program specified
    Failed during: /usr/bin/sudo /usr/bin/xcode-select --switch /Library/Developer/CommandLineTools
    ---- End output of /opt/workstation/.chef/local-mode-cache/cache/homebrew_go < /dev/null ----
    Ran /opt/workstation/.chef/local-mode-cache/cache/homebrew_go < /dev/null returned 1

    Resource Declaration:
    ---------------------
    # In /opt/workstation/.chef/local-mode-cache/cache/cookbooks/homebrew/recipes/default.rb

     49:   execute 'install homebrew' do
     50:     command "#{homebrew_go} < /dev/null"
     51:     environment lazy { { 'HOME' => ::Dir.home(homebrew_owner), 'USER' => homebrew_owner } }
     52:     user homebrew_owner
     53:     not_if { ::File.exist? '/usr/local/bin/brew' }
     54:   end
     55: ensure

After which I added /usr/bin/xcode-select to the list of commands, and the Homebrew install worked (though a later install of a Homebrew cask failed with sudo issues).

Sauraus · 2017-01-20T02:06:41Z

I just ran several kitchen tests on my local mac and they all converged just fine, not a single fault to be found.

discentem · 2017-01-20T14:49:54Z

@Sauraus Did you run rubocop linting though? This is what the failed build indicates.

Sauraus · 2017-01-20T18:10:39Z

@discentem I did but I wasn't going to break the code just to satisfy rubocop BTW. When I run rubocop locally I get 16 offenses and not 6.

Inspecting 24 files
.C.C.C.C..W...C........C

Offenses:

Gemfile:10:1: C: Gem rake should appear before tomlrb in their gem group.
gem 'rake'
^^^^^^^^^^
Gemfile:12:1: C: Gem community_cookbook_releaser should appear before stove in their gem group.
gem 'community_cookbook_releaser'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Rakefile:24:23: C: Avoid comma after the last item of a hash.
        progress: true,
                      ^
libraries/homebrew_mixin.rb:22:1: C: Missing top-level class documentation comment.
class Chef12HomebrewUser
^^^^^
libraries/homebrew_mixin.rb:42:14: C: Align the parameters of a method call if they span more than one line.
             "Homebrew owner is 'root' which is not supported. " \ ...
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
providers/cask.rb:46:1: C: Use alias instead of alias_method at the top level.
alias_method :action_cask, :action_install
^^^^^^^^^^^^
providers/cask.rb:47:1: C: Use alias instead of alias_method at the top level.
alias_method :action_uncask, :action_uninstall
^^^^^^^^^^^^
recipes/default.rb:43:14: W: (...) interpreted as grouped expression.
    variables (
             ^
recipes/default.rb:44:12: C: Avoid using {...} for multi-line blocks.
      lazy {
           ^
recipes/default.rb:45:11: C: Use the new Ruby 1.9 hash syntax.
        { :user => homebrew_owner, :hostname => node['hostname'],
          ^^^^^^^^
recipes/default.rb:45:36: C: Use the new Ruby 1.9 hash syntax.
        { :user => homebrew_owner, :hostname => node['hostname'],
                                   ^^^^^^^^^^^^
recipes/default.rb:46:11: C: Use the new Ruby 1.9 hash syntax.
          :commands => node['homebrew']['sudo']['commands'] }
          ^^^^^^^^^^^^
recipes/default.rb:72:121: C: Line is too long. [151/120]
  only_if { shell_out('/usr/local/bin/brew analytics state', user: homebrew_owner).stdout.include?('enabled') != node['homebrew']['enable-analytics'] }
                                                                                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
resources/cask.rb:5:3: C: Align the parameters of a method call if they span more than one line.
  name_attribute: true, ...
  ^^^^^^^^^^^^^^^^^^^^^
resources/cask.rb:10:3: C: Align the parameters of a method call if they span more than one line.
  kind_of: String
  ^^^^^^^^^^^^^^^
test/integration/default/default_spec.rb:10:23: C: Use 0o for octal literals.
  it { should be_mode 0755 }
                      ^^^^

24 files inspected, 16 offenses detected

Signed-off-by: Antek S. Baranski <antek.baranski@gmail.com>

Sauraus · 2017-01-20T20:18:46Z

@tas50 I've fixed the rubocop warnings, but the spec tests are something entirely different IMO.

andrew-cc · 2017-01-20T20:36:02Z

FWIW, as Sauraus seemed to see, we went the route of just installing Homebrew under its own system user which can do passwordless sudo. Then a “real” user just does e.g. sudo -Hu homebrew brew install whatever, and then we can just set the chef homebrew cookbook user attribute to “homebrew”.

Sauraus · 2017-01-21T00:52:29Z

@andrew-cc saw that yes, still scary to having a user with limitless sudo ;)

andrew-cc · 2017-01-23T15:22:04Z

Since it’s a user that can’t be logged-in to, some other user must use sudo to become them. It’s perhaps a shame in some sense that Homebrew isn’t designed like any other major *nix package manager. In some sense it’s “safer” having everything owned by a different user since it prevents arbitrary user software from modifying /usr/local since it doesn’t have perms.

Sauraus · 2017-01-23T17:37:14Z

Agree with your sentiment.

PS. Don't say any of that to the Homebrew maintainers @Brantone has experience with that...

aramprice · 2017-02-07T19:23:24Z

@Sauraus the spec failures are likely related to this issue #114.

tas50 · 2018-07-24T20:21:20Z

I'm going to close this out at this point because this just isn't an appropriate solution for most users. We try to implement the path of least surprise at Chef and giving blanket sudo rights is not going to be expected or desired by a good chunk of our userbase.

Sauraus force-pushed the master branch from 97b8f0e to 87baa9e Compare October 5, 2016 15:52

ed-flanagan reviewed Nov 29, 2016

View reviewed changes

Sauraus force-pushed the master branch 2 times, most recently from 77956db to 94e9d36 Compare December 9, 2016 01:03

Sauraus force-pushed the master branch 2 times, most recently from e65c5ad to 288cb95 Compare January 20, 2017 02:03

Sauraus force-pushed the master branch from 288cb95 to bb75ef1 Compare January 20, 2017 19:18

Latest brew install needs extended sudo rights

3630e2d

Signed-off-by: Antek S. Baranski <antek.baranski@gmail.com>

Sauraus force-pushed the master branch from bb75ef1 to 3630e2d Compare January 20, 2017 19:23

tas50 closed this Jul 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Latest brew install needs extended sudo rights #109

Latest brew install needs extended sudo rights #109

Sauraus commented Oct 5, 2016

kameghamegha commented Nov 7, 2016

bdangit commented Nov 18, 2016

ed-flanagan Nov 29, 2016

Sauraus Nov 29, 2016

ed-flanagan Nov 29, 2016

Sauraus Nov 29, 2016

ed-flanagan Nov 29, 2016

discentem commented Dec 24, 2016

andrew-cc commented Jan 10, 2017

andrew-cc commented Jan 10, 2017

Sauraus commented Jan 20, 2017

discentem commented Jan 20, 2017

Sauraus commented Jan 20, 2017

Sauraus commented Jan 20, 2017

andrew-cc commented Jan 20, 2017

Sauraus commented Jan 21, 2017

andrew-cc commented Jan 23, 2017

Sauraus commented Jan 23, 2017

aramprice commented Feb 7, 2017

tas50 commented Jul 24, 2018

Latest brew install needs extended sudo rights #109

Latest brew install needs extended sudo rights #109

Conversation

Sauraus commented Oct 5, 2016

kameghamegha commented Nov 7, 2016

bdangit commented Nov 18, 2016

ed-flanagan Nov 29, 2016

Choose a reason for hiding this comment

Sauraus Nov 29, 2016

Choose a reason for hiding this comment

ed-flanagan Nov 29, 2016

Choose a reason for hiding this comment

Sauraus Nov 29, 2016

Choose a reason for hiding this comment

ed-flanagan Nov 29, 2016

Choose a reason for hiding this comment

discentem commented Dec 24, 2016

andrew-cc commented Jan 10, 2017

andrew-cc commented Jan 10, 2017

Sauraus commented Jan 20, 2017

discentem commented Jan 20, 2017

Sauraus commented Jan 20, 2017

Sauraus commented Jan 20, 2017

andrew-cc commented Jan 20, 2017

Sauraus commented Jan 21, 2017

andrew-cc commented Jan 23, 2017

Sauraus commented Jan 23, 2017

aramprice commented Feb 7, 2017

tas50 commented Jul 24, 2018