nix-community / bundix

Generates a Nix expression for your Bundler-managed application. [maintainer=@manveru]
163 stars 55 forks source link

Set gemset.nix file permissions: 0644 #29

Closed ivanbrennan closed 6 years ago

ivanbrennan commented 6 years ago

The tempfile used within save_gemset is created with permissions 0600, and this results in a gemset.nix that's only readable by the file owner.

This can be problematic in certain cases. For example, I used bundix to package a gem in an overlay that's part of my nixos configuration, and is thus owned by root. As a non-root user, I added a nixpkgs-overlays entry to my NIX_PATH and tried to invoke nix-shell with access to the gem. This failed due to a lack of read permissions:

nix-shell -p ruby my-gem
error: opening file '/etc/nixos/path/to/my-gem/gemset.nix': Permission denied

Changing the permissions to 0644 fixes this issue, and matches the Gemfile.lock file permissions.

I'm assuming the 0600 permissions were an artifact of Tempfile.new() rather than a deliberate decision, but let me know if that's not the case.

I tested the FileUtils.chmod call manually. I'm not sure how/whether to write an automated test for this change. If you have a suggestion about how to do so I'd be happy to give it a try.

zimbatm commented 6 years ago

@ivanbrennan just pushed a small simplification, mind testing it out?

ivanbrennan commented 6 years ago

@zimbatm I was hoping to do something like your change, but it doesn't work. In IRB:

003:0 ❯ tempfile = Tempfile.create('foofile', encoding: 'UTF-8', mode: 0644)
=> #<File:/run/user/1000/foofile20180429-14122-1f7lcil>

In the shell:

ls -l /run/user/1000/foofile20180429-14122-1f7lcil
-rw------- 1 ivan users 0 Apr 29 09:37 /run/user/1000/foofile20180429-14122-1f7lcil

The mode argument specifies mode in the sense of append/truncate rather than permissions. Both ::new and ::create have permissions hard-coded to 0600.

The docs also mention that unlike ::new, which produces a Tempfile object, ::create produces a File object. Maybe that doesn't matter in this case -- I don't know the what distinguishes a Tempfile from a File object.

It looks like we either need to run chmod as a separate step, as I was originally doing, or work at a lower level (e.g. calling ::Dir::Tmpname.create directly). At that point though, we'd basically be re-implementing Tempfile. I think chmod is the most sensible choice here.

ivanbrennan commented 6 years ago

It looks like using ::create would also break the subsequent calls to close! and unlink, since those are methods of Tempfile, not File.

zimbatm commented 6 years ago

thanks