Merge branch '221106-chart-links-url-blocking' into 'master'

Add url validation for metrics dashboard chart links

Closes #219074 and #221106

See merge request gitlab-org/gitlab!34888

changelogs/unreleased/221106-chart-links-url-blocking.yml

+---
+title: Block invalid URLs in metrics dashboard chart links
+merge_request: 34888
+author:
+type: added

lib/gitlab/metrics/dashboard/stages/url_validator.rb

@@ -6,8 +6,47 @@ module Gitlab
       module Stages
         class UrlValidator < BaseStage
           def transform!
-            dashboard[:links]&.each do |link|
-              Gitlab::UrlBlocker.validate!(link[:url])
+            validate_dashboard_links(dashboard)
+
+            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
               link[:url] = ''
             end

spec/lib/gitlab/metrics/dashboard/processor_spec.rb

@@ -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'
     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
 
   private

spec/lib/gitlab/metrics/dashboard/stages/url_validator_spec.rb

+# 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