Commit a78abf3a authored by Alexis Reigel's avatar Alexis Reigel

use safer .hooks_for instead of .public_send

with .public_send we can't make sure that the scope on the model
actually exists.
parent ac92d70d
......@@ -17,6 +17,12 @@ module TriggerableHooks
class_methods do
attr_reader :triggerable_hooks
def hooks_for(trigger)
return none unless self::TRIGGERS.keys.include?(trigger)
public_send(trigger) # rubocop:disable GitlabSecurity/PublicSend
end
private
def triggerable_hooks(hooks)
......
......@@ -970,7 +970,7 @@ class Project < ActiveRecord::Base
def execute_hooks(data, hooks_scope = :push_hooks)
run_after_commit_or_now do
hooks.public_send(hooks_scope).each do |hook| # rubocop:disable GitlabSecurity/PublicSend
hooks.hooks_for(hooks_scope).each do |hook|
hook.async_execute(data, hooks_scope.to_s)
end
end
......
......@@ -8,7 +8,7 @@ class SystemHooksService
end
def execute_hooks(data, hooks_scope = :all)
SystemHook.public_send(hooks_scope).find_each do |hook| # rubocop:disable GitlabSecurity/PublicSend
SystemHook.hooks_for(hooks_scope).find_each do |hook|
hook.async_execute(data, 'system_hooks')
end
end
......
......@@ -14,4 +14,22 @@ RSpec.describe TriggerableHooks do
expect(TestableHook).not_to respond_to :tag_push_hooks
end
end
describe '.hooks_for' do
context 'the model has the required trigger scope' do
it 'returns the record' do
hook = TestableHook.create!(url: 'http://example.com', push_events: true)
expect(TestableHook.hooks_for(:push_hooks)).to eq [hook]
end
end
context 'the model does not have the required trigger scope' do
it 'returns an empty relation' do
TestableHook.create!(url: 'http://example.com', push_events: true)
expect(TestableHook.hooks_for(:tag_push_hooks)).to eq []
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment