sous-chefs / homebrew

Development repository for the homebrew cookbook
https://supermarket.chef.io/cookbooks/homebrew
Apache License 2.0
152 stars 136 forks source link

Incorrect permissions on /opt/homebrew-cask and subfolders #52

Closed ringods closed 9 years ago

ringods commented 10 years ago

When installing the java7 cask, I bumped into permission errors on /opt/homebrew-cask folder and subfolders.

STDERR: Error: Permission denied - /opt/homebrew-cask/Caskroom/java7

This folder structure was owned by root, not the Homebrew owner.

Looking at the Pivotal Sprout Cask recipe, I noticed they set the permissions on this folder explicitely to the Homebrew owner, in their case the current user.

The cask recipe needs to be extended with a similar setup. I validated manually that setting the permissions on the folder fixed the installation process of the cask.

jtimberman commented 10 years ago

What group owns the directory? Is it staff? We have a recipe that does this:

directory '/opt/homebrew-cask' do
  mode 0775
end

directory '/opt/homebrew-cask/Caskroom' do
  mode 0775
end

But that particular recipe is in a chef-client run as root via sudo.

I think a potential reason why we wouldn't manage these directories in the recipe is because someone might want to run this as a nonprivileged user, rather than via sudo/root, and they wouldn't have permission to create the directory in /opt, let alone change the ownership of it.

ringods commented 10 years ago

@jtimberman that's why I think that the cookbook should manage these folders, so that a non-priviliged user can install casks after the correct folder permissions are set. I still think this snippet from the Pivotal recipe makes sense:

directory '/opt/homebrew-cask/Caskroom' do
  action :create
  recursive true
  mode '0755'
  owner node['homebrew_owner']
  group 'staff'
end

directory '/opt/homebrew-cask' do
  owner node['homebrew_owner']
end
jtimberman commented 9 years ago

In Chef 12 with the homebrew provider moved over to be the default package provider on OS X, we have an improved helper to determine the homebrew owner.

https://github.com/opscode/chef/blob/master/lib/chef/mixin/homebrew_user.rb

In the homebrew cookbook itself, attributes/default.rb allows setting node['homebrew']['owner'] (rather than homebrew_owner). Perhaps we should attempt to use the Chef mixin, and fall back to the attribute?

moregeek commented 9 years ago

In the homebrew cookbook itself, attributes/default.rb allows setting node['homebrew'] ['owner'](rather than homebrew_owner). Perhaps we should attempt to use the Chef mixin, and fall back to the attribute?

@jtimberman you are right here. the existing attribute should be reused.

To summarize this issue:

When you have a look at the homebrew-cask/lib/cask.rb file, you can see that on the first run after "casking" the "init" method is called to create the folder structure (if it not existing yet). This causes the "wrong" folder permissions on the above directories.

Therefore this permissions should be managed within this cookbook.

I suggest to add a slightly modified version of @ringods code snippet above to the recipes/cask.rb file to handle the permissions properly as set in the attribute.

mkcode commented 9 years ago

+1 on having /opt/homebrew-cask/Caskroom managed by this repo. Ran into this issue and overriding ['homebrew']['owner'] does not fix the problem for me.

Using version 1.9.2 Same issue exists on 1.10.0