Skip to content
This repository
Browse code

atomicc.rb: Don't assume we may chown/chmod a file.

Previously this code just assumed it is capable of changing the file
ownership, both user and group. This will fail in a lot of scenario's
unless:
* The process is run as a superuser (root);
* The owning user and group are already set to the user and group we're
  trying to chown to;
* The user chown'ing only changes the group to another group it is a
  member of.
If either of those conditions are not met the filesystem will simply
deny the operation throwing an error.

It is also not always possible to do a chmod, there might be a SELinux
policy or another limitation preventing the user to change the file
mode. To this end the chmod call has also been added to the rescue
block.

I've also added a little comment above the chmod command that doing a
chmod on a file which has an ACL set will cause the ACL to be
recalculated / modified.
  • Loading branch information...
commit 851f8c10235a0874f5e34b2c7b5544c33f89c022 1 parent 81679ab
Daniele Sluijters daenney authored
2  activesupport/CHANGELOG.md
Source Rendered
@@ -2,6 +2,8 @@
2 2
3 3 * Implement HashWithIndifferentAccess#replace so key? works correctly. *David Graham*
4 4
  5 +* Handle the possible Permission Denied errors atomic.rb might trigger due to its chown and chmod calls. *Daniele Sluijters*
  6 +
5 7 * Hash#extract! returns only those keys that present in the receiver.
6 8
7 9 {:a => 1, :b => 2}.extract!(:a, :x) # => {:a => 1}
9 activesupport/lib/active_support/core_ext/file/atomic.rb
@@ -36,8 +36,13 @@ def self.atomic_write(file_name, temp_dir = Dir.tmpdir)
36 36 FileUtils.mv(temp_file.path, file_name)
37 37
38 38 # Set correct permissions on new file
39   - chown(old_stat.uid, old_stat.gid, file_name)
40   - chmod(old_stat.mode, file_name)
  39 + begin
  40 + chown(old_stat.uid, old_stat.gid, file_name)
  41 + # This operation will affect filesystem ACL's
  42 + chmod(old_stat.mode, file_name)
  43 + rescue Errno::EPERM
  44 + # Changing file ownership failed, moving on.
  45 + end
41 46 end
42 47
43 48 # Private utility method.
4 guides/source/active_support_core_extensions.md
Source Rendered
@@ -3716,7 +3716,9 @@ File.atomic_write(joined_asset_path) do |cache|
3716 3716 end
3717 3717 ```
3718 3718
3719   -To accomplish this `atomic_write` creates a temporary file. That's the file the code in the block actually writes to. On completion, the temporary file is renamed, which is an atomic operation on POSIX systems. If the target file exists `atomic_write` overwrites it and keeps owners and permissions.
  3719 +To accomplish this `atomic_write` creates a temporary file. That's the file the code in the block actually writes to. On completion, the temporary file is renamed, which is an atomic operation on POSIX systems. If the target file exists `atomic_write` overwrites it and keeps owners and permissions. However there are a few cases where `atomic_write` cannot change the file ownership or permissions, this error is caught and skipped over trusting in the user/filesystem to ensure the file is accessible to the processes that need it.
  3720 +
  3721 +NOTE. Due to the chmod operation `atomic_write` performs, if the target file has an ACL set on it this ACL will be recalculated/modified.
3720 3722
3721 3723 WARNING. Note you can't append with `atomic_write`.
3722 3724

0 comments on commit 851f8c1

Please sign in to comment.
Something went wrong with that request. Please try again.