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.

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.

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

Yep, that sounds like it would work well