diff options
| -rw-r--r-- | app/validators/system_hook_url_validator.rb | 6 | ||||
| -rw-r--r-- | doc/security/img/outbound_requests_section_v2.png | bin | 0 -> 21108 bytes | |||
| -rw-r--r-- | doc/security/webhooks.md | 8 | ||||
| -rw-r--r-- | spec/services/web_hook_service_spec.rb | 50 | ||||
| -rw-r--r-- | spec/validators/system_hook_url_validator_spec.rb | 25 |
5 files changed, 43 insertions, 46 deletions
diff --git a/app/validators/system_hook_url_validator.rb b/app/validators/system_hook_url_validator.rb index c8c0007e35b..e482828685d 100644 --- a/app/validators/system_hook_url_validator.rb +++ b/app/validators/system_hook_url_validator.rb @@ -2,7 +2,7 @@ # SystemHookUrlValidator # -# Custom validator specifically for SystemHook URLs. This validator works like AddressableUrlValidator but +# Custom validator specific to SystemHook URLs. This validator works like AddressableUrlValidator but # it blocks urls pointing to localhost or the local network depending on # ApplicationSetting.allow_local_requests_from_system_hooks # @@ -14,8 +14,8 @@ # class SystemHookUrlValidator < AddressableUrlValidator DEFAULT_OPTIONS = { - allow_localhost: true, - allow_local_network: true + allow_localhost: false, + allow_local_network: false }.freeze def initialize(options) diff --git a/doc/security/img/outbound_requests_section_v2.png b/doc/security/img/outbound_requests_section_v2.png Binary files differnew file mode 100644 index 00000000000..4fd3c7d9fce --- /dev/null +++ b/doc/security/img/outbound_requests_section_v2.png diff --git a/doc/security/webhooks.md b/doc/security/webhooks.md index 1194234a295..401df02e5c3 100644 --- a/doc/security/webhooks.md +++ b/doc/security/webhooks.md @@ -34,15 +34,15 @@ to 127.0.0.1, ::1 and 0.0.0.0, as well as IPv4 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 and IPv6 site-local (ffc0::/10) addresses won't be allowed. This behavior can be overridden by enabling the option *"Allow requests to the -local network from hooks and services"* in the *"Outbound requests"* section +local network from web hooks and services"* in the *"Outbound requests"* section inside the Admin area under **Settings** (`/admin/application_settings/network`): - + >**Note:** -*System hooks* are exempt from this protection because they are set up by -admins. +*System hooks* are enabled to make requests to local network by default since they are set up by admins. +However, it can be turned off by disabling *"Allow requests to the local network from system hooks"* option. <!-- ## Troubleshooting diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 8353fade5ea..115599bd1a4 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -19,44 +19,36 @@ describe WebHookService do let(:service_instance) { described_class.new(project_hook, data, :push_hooks) } describe '#initialize' do - context 'when SystemHook' do - context 'when allow_local_requests_from_system_hooks application setting is true' do - it 'allows local requests' do - stub_application_setting(allow_local_requests_from_system_hooks: true) - instance = described_class.new(build(:system_hook), data, :system_hook) + before do + stub_application_setting(setting_name => setting) + end - expect(instance.request_options[:allow_local_requests]).to be_truthy - end + shared_examples_for 'respecting outbound network setting' do + context 'local requests are allowed' do + let(:setting) { true } + + it { expect(hook.request_options[:allow_local_requests]).to be_truthy } end - context 'when allow_local_requests_from_system_hooks application setting is false' do - it 'denies local requests' do - stub_application_setting(allow_local_requests_from_system_hooks: false) - instance = described_class.new(build(:system_hook), data, :system_hook) + context 'local requests are not allowed' do + let(:setting) { false } - expect(instance.request_options[:allow_local_requests]).to be_falsey - end + it { expect(hook.request_options[:allow_local_requests]).to be_falsey } end + end - context 'when ProjectHook' do - context 'when allow_local_requests_from_web_hooks_and_services application setting is true' do - it 'allows local requests' do - stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) - instance = described_class.new(build(:project_hook), data, :project_hook) + context 'when SystemHook' do + let(:setting_name) { :allow_local_requests_from_system_hooks } + let(:hook) { described_class.new(build(:system_hook), data, :system_hook) } - expect(instance.request_options[:allow_local_requests]).to be_truthy - end - end + include_examples 'respecting outbound network setting' + end - context 'when allow_local_requests_from_system_hooks application setting is false' do - it 'denies local requests' do - stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) - instance = described_class.new(build(:project_hook), data, :project_hook) + context 'when ProjectHook' do + let(:setting_name) { :allow_local_requests_from_web_hooks_and_services } + let(:hook) { described_class.new(build(:project_hook), data, :project_hook) } - expect(instance.request_options[:allow_local_requests]).to be_falsey - end - end - end + include_examples 'respecting outbound network setting' end end diff --git a/spec/validators/system_hook_url_validator_spec.rb b/spec/validators/system_hook_url_validator_spec.rb index fc4261666e7..78e95db2b47 100644 --- a/spec/validators/system_hook_url_validator_spec.rb +++ b/spec/validators/system_hook_url_validator_spec.rb @@ -11,43 +11,48 @@ describe SystemHookUrlValidator do subject { validator.validate(badge) } - it 'does not block urls pointing to localhost' do + it 'blocks urls pointing to localhost' do badge.link_url = 'https://127.0.0.1' subject - expect(badge.errors).not_to be_present + expect(badge.errors).to be_present end - it 'does not block urls pointing to the local network' do + it 'blocks urls pointing to the local network' do badge.link_url = 'https://192.168.1.1' subject - expect(badge.errors).not_to be_present + expect(badge.errors).to be_present end end - context 'when local requests are not allowed' do - let(:validator) { described_class.new(attributes: [:link_url], allow_localhost: false, allow_local_network: false) } + context 'when local requests are allowed' do + let(:validator) { described_class.new(attributes: [:link_url]) } let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + let!(:settings) { create(:application_setting) } subject { validator.validate(badge) } - it 'blocks urls pointing to localhost' do + before do + stub_application_setting(allow_local_requests_from_system_hooks: true) + end + + it 'does not block urls pointing to localhost' do badge.link_url = 'https://127.0.0.1' subject - expect(badge.errors).to be_present + expect(badge.errors).not_to be_present end - it 'blocks urls pointing to the local network' do + it 'does not block urls pointing to the local network' do badge.link_url = 'https://192.168.1.1' subject - expect(badge.errors).to be_present + expect(badge.errors).not_to be_present end end end |
