From eaa918e38674b6d4c85601b7a888c13082e791f3 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Fri, 7 Oct 2016 11:48:15 -0700
Subject: [PATCH] Add support checking the permissions of symlink targets to
 the storage directory helper

---
 CHANGELOG.md                                               | 1 +
 .../gitlab/libraries/storage_directory_helper.rb           | 7 ++++---
 spec/chef_helper.rb                                        | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0f59b2817c..50004201cb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ omnibus-gitlab repository.
 
 - Add support for registry debug addr configuration
 - Add support for configuring workhorse's api limiting
+- Update the storage directory helper to check permissions for symlink targets
 - Support specifying a post reconfigure script to run in the docker container
 - Updated cacerts.pem to 2016-09-14 version
 - Add support for nginx status
diff --git a/files/gitlab-cookbooks/gitlab/libraries/storage_directory_helper.rb b/files/gitlab-cookbooks/gitlab/libraries/storage_directory_helper.rb
index b6753147bf..14598ca41e 100644
--- a/files/gitlab-cookbooks/gitlab/libraries/storage_directory_helper.rb
+++ b/files/gitlab-cookbooks/gitlab/libraries/storage_directory_helper.rb
@@ -26,7 +26,7 @@ class StorageDirectoryHelper
   end
 
   def writable?(path)
-    do_shell_out("test -w #{path}", @target_owner).exitstatus == 0
+    do_shell_out("test -w #{path} -a -w $(readlink -f #{path})", @target_owner).exitstatus == 0
   end
 
   def run_command(cmd, use_euid: false, throw_error: true)
@@ -60,13 +60,14 @@ class StorageDirectoryHelper
     # Use stat to return the owner. The root user may not have execute permissions
     # to the directory, but the target_owner will in the success case, so always
     # use the euid to run the command
-    run_command("stat --printf='%U' #{path}", use_euid: true).stdout
+    run_command("stat --printf='%U' $(readlink -f #{path})", use_euid: true).stdout
   end
 
   def run_chown(path)
     # Chown will not work if it's in a root_squash directory, so the only workarounds
     # will be for the admin to manually chown on the nfs server, or use
     # 'no_root_squash' mode in their exports and re-run reconfigure
+    path = File.realpath(path)
     FileUtils.chown(@target_owner, @target_group, path)
   rescue Errno::EPERM => e
     raise(
@@ -103,6 +104,6 @@ class StorageDirectoryHelper
       expect_string << " #{@target_mode}"
     end
 
-    "test -d \"#{path}\" -a \"$(stat --printf='#{format_string}' #{path})\" = '#{expect_string}'"
+    "test -d \"#{path}\" -a \"$(stat --printf='#{format_string}' $(readlink -f #{path}))\" = '#{expect_string}'"
   end
 end
diff --git a/spec/chef_helper.rb b/spec/chef_helper.rb
index 3af1661e32..6e4b1d4b32 100644
--- a/spec/chef_helper.rb
+++ b/spec/chef_helper.rb
@@ -36,7 +36,7 @@ RSpec.configure do |config|
     stub_command(%r{\(test -f /var/opt/gitlab/gitlab-rails/upgrade-status/db-migrate-\h+-\) && \(cat /var/opt/gitlab/gitlab-rails/upgrade-status/db-migrate-\h+- | grep -Fx 0\)}).and_return('')
     stub_command("getenforce | grep Disabled").and_return(true)
     stub_command("semodule -l | grep '^#gitlab-7.2.0-ssh-keygen\\s'").and_return(true)
-    stub_command(%r{test \-d "[^"]\" \-a "\$\(stat \-\-printf='[^']*' /[^\)]*\)" = }).and_return(false)
+    stub_command(%r{test \-d "[^"]\" \-a "\$\(stat \-\-printf='[^']*' \$\(readlink -f /[^\)]*\)\)" = }).and_return(false)
     allow_any_instance_of(Chef::Recipe).to receive(:system).with('/sbin/init --version | grep upstart')
     allow_any_instance_of(Chef::Recipe).to receive(:system).with('systemctl | grep "\-\.mount"')
   end
-- 
GitLab