Accept environment variables from the pre-receive
script.
1. Starting version 2.11, git changed the way the pre-receive flow works. - Previously, the new potential objects would be added to the main repo. If the pre-receive passes, the new objects stay in the repo but are linked up. If the pre-receive fails, the new objects stay orphaned in the repo, and are cleaned up during the next `git gc`. - In 2.11, the new potential objects are added to a temporary "alternate object directory", that git creates for this purpose. If the pre-receive passes, the objects from the alternate object directory are migrated to the main repo. If the pre-receive fails the alternate object directory is simply deleted. 2. In our workflow, the pre-recieve script (in `gitlab-shell) calls the `/allowed` endpoint, which calls out directly to git to perform various checks. These direct calls to git do _not_ have the necessary environment variables set which allow access to the "alternate object directory" (explained above). Therefore these calls to git are not able to access any of the new potential objects to be added during this push. 3. We fix this by accepting the relevant environment variables (GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_OBJECT_DIRECTORY) on the `/allowed` endpoint, and then include these environment variables while calling out to git. 4. This commit includes (whitelisted) these environment variables while making the "force push" check. A `Gitlab::Git::RevList` module is extracted to prevent `ForcePush` from being littered with these checks.
This commit is contained in:
parent
ca6bf62ec1
commit
f82d549d26
7 changed files with 51 additions and 9 deletions
|
@ -52,6 +52,14 @@ module API
|
|||
:push_code
|
||||
]
|
||||
end
|
||||
|
||||
def parse_allowed_environment_variables
|
||||
return if params[:env].blank?
|
||||
|
||||
JSON.parse(params[:env])
|
||||
|
||||
rescue JSON::ParserError
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -32,7 +32,11 @@ module API
|
|||
if wiki?
|
||||
Gitlab::GitAccessWiki.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities)
|
||||
else
|
||||
Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities)
|
||||
Gitlab::GitAccess.new(actor,
|
||||
project,
|
||||
protocol,
|
||||
authentication_abilities: ssh_authentication_abilities,
|
||||
env: parse_allowed_environment_variables)
|
||||
end
|
||||
|
||||
access_status = access.check(params[:action], params[:changes])
|
||||
|
|
|
@ -3,11 +3,12 @@ module Gitlab
|
|||
class ChangeAccess
|
||||
attr_reader :user_access, :project
|
||||
|
||||
def initialize(change, user_access:, project:)
|
||||
def initialize(change, user_access:, project:, env: {})
|
||||
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
|
||||
@branch_name = Gitlab::Git.branch_name(@ref)
|
||||
@user_access = user_access
|
||||
@project = project
|
||||
@env = env
|
||||
end
|
||||
|
||||
def exec
|
||||
|
@ -68,7 +69,7 @@ module Gitlab
|
|||
end
|
||||
|
||||
def forced_push?
|
||||
Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
|
||||
Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev, env: @env)
|
||||
end
|
||||
|
||||
def matching_merge_request?
|
||||
|
|
|
@ -1,14 +1,14 @@
|
|||
module Gitlab
|
||||
module Checks
|
||||
class ForcePush
|
||||
def self.force_push?(project, oldrev, newrev)
|
||||
def self.force_push?(project, oldrev, newrev, env: {})
|
||||
return false if project.empty_repo?
|
||||
|
||||
# Created or deleted branch
|
||||
if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev)
|
||||
false
|
||||
else
|
||||
missed_ref, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list --max-count=1 #{oldrev} ^#{newrev}))
|
||||
missed_ref, _ = Gitlab::Git::RevList.new(oldrev, newrev, project: project, env: env).execute
|
||||
missed_ref.present?
|
||||
end
|
||||
end
|
||||
|
|
28
lib/gitlab/git/rev_list.rb
Normal file
28
lib/gitlab/git/rev_list.rb
Normal file
|
@ -0,0 +1,28 @@
|
|||
# Call out to the `git rev-list` command
|
||||
|
||||
module Gitlab
|
||||
module Git
|
||||
class RevList
|
||||
def initialize(oldrev, newrev, project:, env: nil)
|
||||
@args = [Gitlab.config.git.bin_path,
|
||||
"--git-dir=#{project.repository.path_to_repo}",
|
||||
"rev-list",
|
||||
"--max-count=1",
|
||||
oldrev,
|
||||
"^#{newrev}"]
|
||||
|
||||
@env = env.slice(*allowed_environment_variables)
|
||||
end
|
||||
|
||||
def execute
|
||||
Gitlab::Popen.popen(@args, nil, @env.slice(*allowed_environment_variables))
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def allowed_environment_variables
|
||||
%w(GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_OBJECT_DIRECTORY)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -17,12 +17,13 @@ module Gitlab
|
|||
|
||||
attr_reader :actor, :project, :protocol, :user_access, :authentication_abilities
|
||||
|
||||
def initialize(actor, project, protocol, authentication_abilities:)
|
||||
def initialize(actor, project, protocol, authentication_abilities:, env: {})
|
||||
@actor = actor
|
||||
@project = project
|
||||
@protocol = protocol
|
||||
@authentication_abilities = authentication_abilities
|
||||
@user_access = UserAccess.new(user, project: project)
|
||||
@env = env
|
||||
end
|
||||
|
||||
def check(cmd, changes)
|
||||
|
@ -103,7 +104,7 @@ module Gitlab
|
|||
end
|
||||
|
||||
def change_access_check(change)
|
||||
Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec
|
||||
Checks::ChangeAccess.new(change, user_access: user_access, project: project, env: @env).exec
|
||||
end
|
||||
|
||||
def protocol_allowed?
|
||||
|
|
|
@ -5,13 +5,13 @@ module Gitlab
|
|||
module Popen
|
||||
extend self
|
||||
|
||||
def popen(cmd, path = nil)
|
||||
def popen(cmd, path = nil, vars = {})
|
||||
unless cmd.is_a?(Array)
|
||||
raise "System commands must be given as an array of strings"
|
||||
end
|
||||
|
||||
path ||= Dir.pwd
|
||||
vars = { "PWD" => path }
|
||||
vars['PWD'] = path
|
||||
options = { chdir: path }
|
||||
|
||||
unless File.directory?(path)
|
||||
|
|
Loading…
Reference in a new issue