Merge branch 'fix-csp-initialization' into 'master'

Move default CSP initialization out of 1_settings.rb

See merge request gitlab-org/gitlab!68091

config/initializers/1_settings.rb

@@ -210,7 +210,7 @@ Settings.gitlab.default_projects_features['visibility_level']   = Settings.__sen
 Settings.gitlab['domain_allowlist'] ||= []
 Settings.gitlab['import_sources'] ||= Gitlab::ImportSources.values
 Settings.gitlab['trusted_proxies'] ||= []
-Settings.gitlab['content_security_policy'] ||= Gitlab::ContentSecurityPolicy::ConfigLoader.default_settings_hash(Settings.gitlab['cdn_host'])
+Settings.gitlab['content_security_policy'] ||= {}
 Settings.gitlab['allowed_hosts'] ||= []
 Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config', 'no_todos_messages.yml'))
 Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil?

config/initializers/content_security_policy.rb

@@ -2,11 +2,16 @@
 
 csp_settings = Settings.gitlab.content_security_policy
 
+csp_settings['enabled'] = Gitlab::ContentSecurityPolicy::ConfigLoader.default_enabled if csp_settings['enabled'].nil?
+csp_settings['report_only'] = false if csp_settings['report_only'].nil?
+csp_settings['directives'] ||= {}
+
 if csp_settings['enabled']
+  csp_settings['directives'] = ::Gitlab::ContentSecurityPolicy::ConfigLoader.default_directives if csp_settings['directives'].empty?
+
   # See https://guides.rubyonrails.org/security.html#content-security-policy
   Rails.application.config.content_security_policy do |policy|
-    directives = csp_settings.fetch('directives', {})
-    loader = ::Gitlab::ContentSecurityPolicy::ConfigLoader.new(directives)
+    loader = ::Gitlab::ContentSecurityPolicy::ConfigLoader.new(csp_settings['directives'].to_h)
     loader.load(policy)
   end
 

lib/gitlab/content_security_policy/config_loader.rb

@@ -7,11 +7,12 @@ module Gitlab
                       form_action frame_ancestors frame_src img_src manifest_src
                       media_src object_src report_uri script_src style_src worker_src).freeze
 
-      def self.default_settings_hash(cdn_host)
-        settings_hash = {
-          'enabled' => Rails.env.development? || Rails.env.test?,
-          'report_only' => false,
-          'directives' => {
+      def self.default_enabled
+        Rails.env.development? || Rails.env.test?
+      end
+
+      def self.default_directives
+        directives = {
           'default_src' => "'self'",
           'base_uri' => "'self'",
           'connect_src' => "'self'",
@@ -28,19 +29,18 @@ module Gitlab
           'object_src' => "'none'",
           'report_uri' => nil
         }
-        }
 
         # frame-src was deprecated in CSP level 2 in favor of child-src
         # CSP level 3 "undeprecated" frame-src and browsers fall back on child-src if it's missing
         # However Safari seems to read child-src first so we'll just keep both equal
-        settings_hash['directives']['child_src'] = settings_hash['directives']['frame_src']
+        directives['child_src'] = directives['frame_src']
 
-        allow_webpack_dev_server(settings_hash) if Rails.env.development?
-        allow_cdn(settings_hash, cdn_host) if cdn_host.present?
-        allow_customersdot(settings_hash) if Rails.env.development? && ENV['CUSTOMER_PORTAL_URL'].present?
-        allow_sentry(settings_hash) if Gitlab.config.sentry&.enabled && Gitlab.config.sentry&.clientside_dsn
+        allow_webpack_dev_server(directives) if Rails.env.development?
+        allow_cdn(directives, Settings.gitlab.cdn_host) if Settings.gitlab.cdn_host.present?
+        allow_customersdot(directives) if Rails.env.development? && ENV['CUSTOMER_PORTAL_URL'].present?
+        allow_sentry(directives) if Gitlab.config.sentry&.enabled && Gitlab.config.sentry&.clientside_dsn
 
-        settings_hash
+        directives
       end
 
       def initialize(csp_directives)
@@ -67,37 +67,37 @@ module Gitlab
         arguments.strip.split(' ').map(&:strip)
       end
 
-      def self.allow_webpack_dev_server(settings_hash)
+      def self.allow_webpack_dev_server(directives)
         secure = Settings.webpack.dev_server['https']
         host_and_port = "#{Settings.webpack.dev_server['host']}:#{Settings.webpack.dev_server['port']}"
         http_url = "#{secure ? 'https' : 'http'}://#{host_and_port}"
         ws_url = "#{secure ? 'wss' : 'ws'}://#{host_and_port}"
 
-        append_to_directive(settings_hash, 'connect_src', "#{http_url} #{ws_url}")
+        append_to_directive(directives, 'connect_src', "#{http_url} #{ws_url}")
       end
 
-      def self.allow_cdn(settings_hash, cdn_host)
-        append_to_directive(settings_hash, 'script_src', cdn_host)
-        append_to_directive(settings_hash, 'style_src', cdn_host)
-        append_to_directive(settings_hash, 'font_src', cdn_host)
+      def self.allow_cdn(directives, cdn_host)
+        append_to_directive(directives, 'script_src', cdn_host)
+        append_to_directive(directives, 'style_src', cdn_host)
+        append_to_directive(directives, 'font_src', cdn_host)
       end
 
-      def self.append_to_directive(settings_hash, directive, text)
-        settings_hash['directives'][directive] = "#{settings_hash['directives'][directive]} #{text}".strip
+      def self.append_to_directive(directives, directive, text)
+        directives[directive] = "#{directives[directive]} #{text}".strip
       end
 
-      def self.allow_customersdot(settings_hash)
+      def self.allow_customersdot(directives)
         customersdot_host = ENV['CUSTOMER_PORTAL_URL']
 
-        append_to_directive(settings_hash, 'frame_src', customersdot_host)
+        append_to_directive(directives, 'frame_src', customersdot_host)
       end
 
-      def self.allow_sentry(settings_hash)
+      def self.allow_sentry(directives)
         sentry_dsn = Gitlab.config.sentry.clientside_dsn
         sentry_uri = URI(sentry_dsn)
         sentry_uri.user = nil
 
-        append_to_directive(settings_hash, 'connect_src', sentry_uri.to_s)
+        append_to_directive(directives, 'connect_src', sentry_uri.to_s)
       end
     end
   end

spec/lib/gitlab/content_security_policy/config_loader_spec.rb

@@ -4,7 +4,6 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
   let(:policy) { ActionDispatch::ContentSecurityPolicy.new }
-  let(:cdn_host) { nil }
   let(:csp_config) do
     {
       enabled: true,
@@ -20,14 +19,28 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
     }
   end
 
-  describe '.default_settings_hash' do
-    let(:settings) { described_class.default_settings_hash(cdn_host) }
+  describe '.default_enabled' do
+    let(:enabled) { described_class.default_enabled }
 
-    it 'returns defaults for all keys' do
-      expect(settings['enabled']).to be_truthy
-      expect(settings['report_only']).to be_falsey
+    it 'is enabled' do
+      expect(enabled).to be_truthy
+    end
 
-      directives = settings['directives']
+    context 'when in production' do
+      before do
+        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
+      end
+
+      it 'is disabled' do
+        expect(enabled).to be_falsey
+      end
+    end
+  end
+
+  describe '.default_directives' do
+    let(:directives) { described_class.default_directives }
+
+    it 'returns default directives' do
       directive_names = (described_class::DIRECTIVES - ['report_uri'])
       directive_names.each do |directive|
         expect(directives.has_key?(directive)).to be_truthy
@@ -39,22 +52,12 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
       expect(directives['child_src']).to eq(directives['frame_src'])
     end
 
-    context 'when in production' do
+    context 'when CDN host is defined' do
       before do
-        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
-      end
-
-      it 'is disabled' do
-        expect(settings['enabled']).to be_falsey
-      end
+        stub_config_setting(cdn_host: 'https://example.com')
       end
 
-    context 'when CDN host is defined' do
-      let(:cdn_host) { 'https://example.com' }
-
       it 'adds CDN host to CSP' do
-        directives = settings['directives']
-
         expect(directives['script_src']).to eq("'strict-dynamic' 'self' 'unsafe-inline' 'unsafe-eval' https://www.google.com/recaptcha/ https://www.recaptcha.net https://apis.google.com https://example.com")
         expect(directives['style_src']).to eq("'self' 'unsafe-inline' https://example.com")
         expect(directives['font_src']).to eq("'self' https://example.com")
@@ -67,8 +70,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
       end
 
       it 'adds sentry path to CSP without user' do
-        directives = settings['directives']
-
         expect(directives['connect_src']).to eq("'self' dummy://example.com/43")
       end
     end
@@ -84,8 +85,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         end
 
         it 'does not add CUSTOMER_PORTAL_URL to CSP' do
-          directives = settings['directives']
-
           expect(directives['frame_src']).to eq("'self' https://www.google.com/recaptcha/ https://www.recaptcha.net/ https://content.googleapis.com https://content-compute.googleapis.com https://content-cloudbilling.googleapis.com https://content-cloudresourcemanager.googleapis.com")
         end
       end
@@ -96,8 +95,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         end
 
         it 'adds CUSTOMER_PORTAL_URL to CSP' do
-          directives = settings['directives']
-
           expect(directives['frame_src']).to eq("'self' https://www.google.com/recaptcha/ https://www.recaptcha.net/ https://content.googleapis.com https://content-compute.googleapis.com https://content-cloudbilling.googleapis.com https://content-cloudresourcemanager.googleapis.com https://customers.example.com")
         end
       end