New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set gemset.nix file permissions: 0644 #29

Merged
merged 3 commits into from Apr 29, 2018

Conversation

Projects
None yet
2 participants
@ivanbrennan

ivanbrennan commented Apr 29, 2018

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.

ivanbrennan and others added some commits Apr 29, 2018

set gemset.nix file permissions to 0644
Set the permissions on gemset.nix to match those of the other files
(e.g. Gemfile.lock). Allowing read access to users other than the file
owner is important if bundix is being used with the intent of copying
the resulting derivation to a root-owned overlay as part of a nixos
configuration. Otherwise, if a non-root user tries to invoke a nix-shell
with access to the resulting derivation, they'll fail due to a lack of
read permissions.
@zimbatm

This comment has been minimized.

Collaborator

zimbatm commented Apr 29, 2018

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

@ivanbrennan

This comment has been minimized.

ivanbrennan commented Apr 29, 2018

@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

This comment has been minimized.

ivanbrennan commented Apr 29, 2018

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

@zimbatm zimbatm merged commit 87bffd9 into manveru:master Apr 29, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@zimbatm

This comment has been minimized.

Collaborator

zimbatm commented Apr 29, 2018

thanks

@ivanbrennan ivanbrennan deleted the ivanbrennan:gemset-permissions branch Apr 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment