Rails core - Nov 2011 - atomic_write's chown breaks multi-user access

Some people may want to run their rails server as one user, deploy
their app as another user, run some rake tasks manually or via cron
that also write to the same file system as another user, etc.  There
are a variety of reasons one may want to do this kind of separation,
including security.  You''d think this should be easy under Unix with a
common group they all share, a sane 002 umask, and a group sticky bit
for the shared-write-access areas of the app.  But it''s not, because
atomic_write breaks this scenario.

Here''s what atomic_write does currently:

1. It writes a new temp file
2. It checks the permissions and ownership of the old file
3. It moves the new temp file over the old file
4. It changes permissions and ownerships of the new file to match the
old one.

The problem is that you can only change the ownership of a file if you
are root (and nobody runs their rails app as root, right?), or if
you''re changing it to yourself and it''s already owned by
yourself (in
which case, it''s a no-op waste of time).  Otherwise you get an
"Operation not permitted" error.

Therefore, since it''s not doing any good anyway, I propose we delete
the chown line in atomic_write, to allow multi-user access in the
above scenario.  We can keep the chmod line as is of course.

Here''s a code snippet from:
https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/file/atomic.rb

    # Overwrite original file with temp file
    FileUtils.mv(temp_file.path, file_name)

    # Set correct permissions on new file
    chown(old_stat.uid, old_stat.gid, file_name)  ##### <-- delete
this line
    chmod(old_stat.mode, file_name)

I am happy to send a pull request on github if people seem to like
this idea.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> # Set correct permissions on new file
> chown(old_stat.uid, old_stat.gid, file_name) ##### <-- delete
> this line
> 
> 

This line was added deliberately.  Prior to its inclusion the files had the
ownership and group information of the file returned by Tempfile, which was
almost always wrong (e.g. the www-data user couldn''t read it).  Now
those values get copied across from the directory in question which means that
you can safely use atomic_write to generate assets that are served by your web
server, you just need to make that directory''s group www-data and add
the running user as a member of that group.

I''ve no idea why you want to separate deploying and running users but
you should be able to work around this by setting the ownership of the relevant
directory at deploy time to the values you want rails to maintain.  Removing
that line seems much more likely to break things for the majority.

--
Cheers,

Koz
 

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Nov 20, 12:47 pm, Michael Koziarski <mich...@koziarski.com>
wrote:
> > chown(old_stat.uid, old_stat.gid, file_name)
>
> This line was added deliberately.
I think I now see the flaw in my original logic, thanks.  How about
this instead:  change the line to:

    chown(nil, old_stat.gid, file_name)

The reason being: no platform ever allows you to change the uid,
unless you''re running as root, and nobody runs their rails servers as
root, right?  But platforms do allow you to change the gid as long as
you are a member, and that''s still useful for all the reasons you
outlined.

Dave

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Yep, that sounds like it would work well

-- 
Cheers,

Koz


On Monday, 21 November 2011 at 10:57 AM, dburry wrote:
> On Nov 20, 12:47 pm, Michael Koziarski <mich...@koziarski.com
(http://koziarski.com)> wrote:
> > > chown(old_stat.uid, old_stat.gid, file_name)
> > 
> > 
> > This line was added deliberately.
> 
> I think I now see the flaw in my original logic, thanks. How about
> this instead: change the line to:
> 
> chown(nil, old_stat.gid, file_name)
> 
> The reason being: no platform ever allows you to change the uid,
> unless you''re running as root, and nobody runs their rails servers
as
> root, right? But platforms do allow you to change the gid as long as
> you are a member, and that''s still useful for all the reasons you
> outlined.
> 
> Dave
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
> To post to this group, send email to rubyonrails-core@googlegroups.com
(mailto:rubyonrails-core@googlegroups.com).
> To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
(mailto:rubyonrails-core+unsubscribe@googlegroups.com).
> For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.
> 
> 

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.