From 61c06c5e1ae87914343312b956d5b289d568b71f Mon Sep 17 00:00:00 2001 From: Vinnie Okada Date: Sun, 15 Mar 2015 12:54:36 -0600 Subject: Change permissions on backup files Use more restrictive permissions for backup tar files and for the db, uploads, and repositories directories inside the tar files. --- CHANGELOG | 1 + lib/backup/manager.rb | 18 +++++++--- spec/tasks/gitlab/backup_rake_spec.rb | 63 +++++++++++++++++++++++++++++++---- 3 files changed, 70 insertions(+), 12 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c4e47346fd8..4787117cbbc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -34,6 +34,7 @@ v 7.9.0 (unreleased) - Add a service to send updates to an Irker gateway (Romain Coltel) - Add brakeman (security scanner for Ruby on Rails) - Slack username and channel options + - Restrict permissions on backup files - Add grouped milestones from all projects to dashboard. - Web hook sends pusher email as well as commiter - Add Bitbucket omniauth provider. diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index ab8db4e9837..1a4f28d106d 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -11,22 +11,28 @@ module Backup s[:tar_version] = tar_version tar_file = "#{s[:backup_created_at].to_i}_gitlab_backup.tar" + orig_pwd = Dir.pwd Dir.chdir(Gitlab.config.backup.path) 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} ... " + 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) + Dir.chdir(orig_pwd) end def upload(tar_file) @@ -51,11 +57,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 diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 60942cc95fc..8a411b7720a 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -10,17 +10,17 @@ describe 'gitlab:app namespace rake task' do Rake::Task.define_task :environment end + def run_rake_task(task_name) + Rake::Task[task_name].reenable + Rake.application.invoke_task task_name + end + describe 'backup_restore' do before do # avoid writing task output to spec progress allow($stdout).to receive :write end - let :run_rake_task do - Rake::Task["gitlab:backup:restore"].reenable - Rake.application.invoke_task "gitlab:backup:restore" - end - context 'gitlab version' do before do Dir.stub glob: [] @@ -36,7 +36,9 @@ describe 'gitlab:app namespace rake task' do it 'should fail on mismatch' do YAML.stub load_file: {gitlab_version: "not #{gitlab_version}" } - expect { run_rake_task }.to raise_error SystemExit + expect { run_rake_task('gitlab:backup:restore') }.to( + raise_error SystemExit + ) end it 'should invoke restoration on mach' do @@ -44,9 +46,56 @@ describe 'gitlab:app namespace rake task' do expect(Rake::Task["gitlab:backup:db:restore"]).to receive :invoke expect(Rake::Task["gitlab:backup:repo:restore"]).to receive :invoke expect(Rake::Task["gitlab:shell:setup"]).to receive :invoke - expect { run_rake_task }.to_not raise_error + expect { run_rake_task('gitlab:backup:restore') }.to_not raise_error end end end # backup_restore task + + describe 'backup_create' do + def tars_glob + Dir.glob(File.join(Gitlab.config.backup.path, '*_gitlab_backup.tar')) + end + + before :all do + # Record the existing backup tars so we don't touch them + existing_tars = tars_glob + + # Redirect STDOUT and run the rake task + orig_stdout = $stdout + $stdout = StringIO.new + run_rake_task('gitlab:backup:create') + $stdout = orig_stdout + + @backup_tar = (tars_glob - existing_tars).first + end + + after :all do + FileUtils.rm(@backup_tar) + end + + it 'should set correct permissions on the tar file' do + expect(File.exist?(@backup_tar)).to be_truthy + expect(File::Stat.new(@backup_tar).mode.to_s(8)).to eq('100600') + end + + it 'should set correct permissions on the tar contents' do + tar_contents, exit_status = Gitlab::Popen.popen( + %W{tar -tvf #{@backup_tar} db uploads repositories} + ) + expect(exit_status).to eq(0) + expect(tar_contents).to match('db/') + expect(tar_contents).to match('uploads/') + expect(tar_contents).to match('repositories/') + expect(tar_contents).not_to match(/^.{4,9}[rwx]/) + end + + it 'should delete temp directories' do + temp_dirs = Dir.glob( + File.join(Gitlab.config.backup.path, '{db,repositories,uploads}') + ) + + expect(temp_dirs).to be_empty + end + end # backup_create task end # gitlab:app namespace -- cgit v1.2.1 From abbea9a0daa009fff4efcee0743307ae27e49d65 Mon Sep 17 00:00:00 2001 From: Vinnie Okada Date: Thu, 19 Mar 2015 19:24:25 -0600 Subject: Move backup permission changes to version 7.10 --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 4787117cbbc..2bdcfa9e609 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.10.0 (unreleased) + - Restrict permissions on backup files v 7.9.0 (unreleased) - Add HipChat integration documentation (Stan Hu) @@ -34,7 +35,6 @@ v 7.9.0 (unreleased) - Add a service to send updates to an Irker gateway (Romain Coltel) - Add brakeman (security scanner for Ruby on Rails) - Slack username and channel options - - Restrict permissions on backup files - Add grouped milestones from all projects to dashboard. - Web hook sends pusher email as well as commiter - Add Bitbucket omniauth provider. -- cgit v1.2.1 From 06aafb73640da21a4277961c5c6da61496f0e8db Mon Sep 17 00:00:00 2001 From: Vinnie Okada Date: Thu, 19 Mar 2015 19:24:57 -0600 Subject: Call chdir() with a block --- lib/backup/manager.rb | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index 1a4f28d106d..c6087830b40 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -11,28 +11,27 @@ module Backup s[:tar_version] = tar_version tar_file = "#{s[:backup_created_at].to_i}_gitlab_backup.tar" - orig_pwd = Dir.pwd - 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}) - FileUtils.chmod_R(0700, %w{db uploads repositories}) + # 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) - # 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' + upload(tar_file) end - File.umask(orig_umask) - - upload(tar_file) - Dir.chdir(orig_pwd) end def upload(tar_file) -- cgit v1.2.1