Commit 63b1e53e authored by Kirstie Cook's avatar Kirstie Cook Committed by Sean McGivern

Add url validation for metrics dashboard chart links

parent cc7052f6
---
title: Block invalid URLs in metrics dashboard chart links
merge_request: 34888
author:
type: added
...@@ -6,8 +6,47 @@ module Gitlab ...@@ -6,8 +6,47 @@ module Gitlab
module Stages module Stages
class UrlValidator < BaseStage class UrlValidator < BaseStage
def transform! def transform!
dashboard[:links]&.each do |link| validate_dashboard_links(dashboard)
Gitlab::UrlBlocker.validate!(link[:url])
validate_chart_links(dashboard)
end
private
def blocker_args
{
schemes: %w(http https),
ports: [],
allow_localhost: allow_setting_local_requests?,
allow_local_network: allow_setting_local_requests?,
ascii_only: false,
enforce_user: false,
enforce_sanitization: false,
dns_rebind_protection: true
}
end
def allow_setting_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
def validate_dashboard_links(dashboard)
validate_links(dashboard[:links])
end
def validate_chart_links(dashboard)
dashboard[:panel_groups].each do |panel_group|
panel_group[:panels].each do |panel|
validate_links(panel[:links])
end
end
end
def validate_links(links)
links&.each do |link|
next unless link.is_a? Hash
Gitlab::UrlBlocker.validate!(link[:url], blocker_args)
rescue Gitlab::UrlBlocker::BlockedUrlError rescue Gitlab::UrlBlocker::BlockedUrlError
link[:url] = '' link[:url] = ''
end end
......
...@@ -202,27 +202,6 @@ describe Gitlab::Metrics::Dashboard::Processor do ...@@ -202,27 +202,6 @@ describe Gitlab::Metrics::Dashboard::Processor do
it_behaves_like 'errors with message', 'Each "metric" must define one of :query or :query_range' it_behaves_like 'errors with message', 'Each "metric" must define one of :query or :query_range'
end end
describe 'validating links' do
context 'when the links contain a blocked url' do
let(:dashboard_yml_links) do
[{ 'url' => 'http://1.1.1.1.1' }, { 'url' => 'https://gitlab.com' }]
end
let(:expected) do
[{ url: '' }, { url: 'https://gitlab.com' }]
end
before do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
dashboard_yml['links'] = dashboard_yml_links
end
it 'replaces the blocked url with an empty string' do
expect(dashboard[:links]).to eq(expected)
end
end
end
end end
private private
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Metrics::Dashboard::Stages::UrlValidator do
let(:project) { build_stubbed(:project) }
describe '#transform!' do
context 'when the links contain a blocked url' do
let(:dashboard) do
{
dashboard: "Test Dashboard",
links: [
{ url: "http://1.1.1.1.1" },
{ url: "https://gitlab.com" },
{ url: "http://0.0.0.0" }
],
panel_groups: [
{
group: "Group A",
panels: [
{
title: "Super Chart A1",
type: "area-chart",
y_label: "y_label",
metrics: [
{
id: "metric_a1",
query_range: "query",
unit: "unit",
label: "Legend Label"
}
],
links: [
{ url: "http://1.1.1.1.1" },
{ url: "https://gitlab.com" },
{ url: "http://0.0.0.0" }
]
}
]
}
]
}
end
let(:expected) do
[{ url: '' }, { url: 'https://gitlab.com' }, { url: 'http://0.0.0.0' }]
end
let(:transform!) { described_class.new(project, dashboard, nil).transform! }
before do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
end
context 'dashboard related links' do
it 'replaces the blocked url with an empty string' do
transform!
expect(dashboard[:links]).to eq(expected)
end
end
context 'chart links' do
it 'replaces the blocked url with an empty string' do
transform!
result = dashboard.dig(:panel_groups, 0, :panels, 0, :links)
expect(result).to eq(expected)
end
end
context 'when local requests are not allowed' do
before do
stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
end
let(:expected) do
[{ url: '' }, { url: 'https://gitlab.com' }, { url: '' }]
end
it 'replaces the blocked url with an empty string' do
transform!
expect(dashboard[:links]).to eq(expected)
end
end
context 'when the links are an array of strings instead of hashes' do
before do
dashboard[:links] = dashboard[:links].map(&:values)
end
it 'prevents an invalid link definition from erroring out' do
expect { transform! }.not_to raise_error
end
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