Merge branch 'backup-permissions' into 'master'

Change permissions on backup files - #2

Use more restrictive permissions for backup tar files and for the db, uploads, and repositories directories inside the tar files.  See #1894.  Now the backup task recursively `chmod`s the `db/`, `uploads/`, and `repositories/` folders with 0700 permissions, and the tar file is created as 0600.

This is a followup to !1703, which was reverted because it broke Rspec tests.  The test failures were due to the rake task changing directories and not changing back, which I fixed with this commit.

cc @sytse

See merge request !1716

1 files changed, 25 insertions, 18 deletions

diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb
index ab8db4e9837..c6087830b40 100644
--- a/lib/backup/manager.rb
+++ b/lib/backup/manager.rb
@@ -11,22 +11,27 @@ module Backup
       s[:tar_version]        = tar_version
       tar_file = "#{s[:backup_created_at].to_i}_gitlab_backup.tar"
 
-      Dir.chdir(Gitlab.config.backup.path)
+      Dir.chdir(Gitlab.config.backup.path) do
+        File.open("#{Gitlab.config.backup.path}/backup_information.yml",
+                  "w+") do |file|
+          file << s.to_yaml.gsub(/^---\n/,'')
+        end
 
-      File.open("#{Gitlab.config.backup.path}/backup_information.yml", "w+") do |file|
-        file << s.to_yaml.gsub(/^---\n/,'')
-      end
+        FileUtils.chmod_R(0700, %w{db uploads repositories})
 
-      # create archive
-      $progress.print "Creating backup archive: #{tar_file} ... "
-      if Kernel.system('tar', '-cf', tar_file, *BACKUP_CONTENTS)
-        $progress.puts "done".green
-      else
-        puts "creating archive #{tar_file} failed".red
-        abort 'Backup failed'
-      end
+        # create archive
+        $progress.print "Creating backup archive: #{tar_file} ... "
+        orig_umask = File.umask(0077)
+        if Kernel.system('tar', '-cf', tar_file, *BACKUP_CONTENTS)
+          $progress.puts "done".green
+        else
+          puts "creating archive #{tar_file} failed".red
+          abort 'Backup failed'
+        end
+        File.umask(orig_umask)
 
-      upload(tar_file)
+        upload(tar_file)
+      end
     end
 
     def upload(tar_file)
@@ -51,11 +56,13 @@ module Backup
 
     def cleanup
       $progress.print "Deleting tmp directories ... "
-      if Kernel.system('rm', '-rf', *BACKUP_CONTENTS)
-        $progress.puts "done".green
-      else
-        puts "deleting tmp directory failed".red
-        abort 'Backup failed'
+      BACKUP_CONTENTS.each do |dir|
+        if FileUtils.rm_rf(File.join(Gitlab.config.backup.path, dir))
+          $progress.puts "done".green
+        else
+          puts "deleting tmp directory '#{dir}' failed".red
+          abort 'Backup failed'
+        end
       end
     end