FEATURE: allow plugins and themes to extend the default CSP (#6704)

* FEATURE: allow plugins and themes to extend the default CSP

For plugins:

```
extend_content_security_policy(
  script_src: ['https://domain.com/script.js', 'https://your-cdn.com/'],
  style_src: ['https://domain.com/style.css']
)
```

For themes and components:

```
extend_content_security_policy:
  type: list
  default: "script_src:https://domain.com/|style_src:https://domain.com"
```

* clear CSP base url before each test

we have a test that stubs `Rails.env.development?` to true

* Only allow extending directives that core includes, for now

app/models/theme_field.rb

@@ -249,6 +249,7 @@ COMPILED
     theme.clear_cached_settings!
 
     Stylesheet::Manager.clear_theme_cache! if self.name.include?("scss")
+    CSP::Extension.clear_theme_extensions_cache! if name == 'yaml'
 
     # TODO message for mobile vs desktop
     MessageBus.publish "/header-change/#{theme.id}", self.value if theme && self.name == "header"

app/models/theme_setting.rb

@@ -10,6 +10,7 @@ class ThemeSetting < ActiveRecord::Base
     theme.remove_from_cache!
     theme.theme_fields.update_all(value_baked: nil)
     SvgSprite.expire_cache if self.name.to_s.include?("_icon")
+    CSP::Extension.clear_theme_extensions_cache! if name.to_s == CSP::Extension::THEME_SETTING
   end
 
   def self.types

config/application.rb

@@ -204,7 +204,7 @@ module Discourse
       config.middleware.insert_after Rack::MethodOverride, Middleware::EnforceHostname
     end
 
-    require 'content_security_policy'
+    require 'content_security_policy/middleware'
     config.middleware.swap ActionDispatch::ContentSecurityPolicy::Middleware, ContentSecurityPolicy::Middleware
 
     require 'middleware/discourse_public_exceptions'

lib/content_security_policy.rb

@@ -1,107 +1,28 @@
 # frozen_string_literal: true
-require_dependency 'global_path'
+require_dependency 'content_security_policy/builder'
+require_dependency 'content_security_policy/extension'
 
 class ContentSecurityPolicy
-  include GlobalPath
+  class << self
-
+    def policy
-  class Middleware
+      new.build
-    def initialize(app)
-      @app = app
     end
 
-    def call(env)
+    def base_url
-      request = Rack::Request.new(env)
+      @base_url || Discourse.base_url
-      _, headers, _ = response = @app.call(env)
-
-      return response unless html_response?(headers) && ContentSecurityPolicy.enabled?
-
-      policy = ContentSecurityPolicy.new(request).build
-      headers['Content-Security-Policy'] = policy if SiteSetting.content_security_policy
-      headers['Content-Security-Policy-Report-Only'] = policy if SiteSetting.content_security_policy_report_only
-
-      response
     end
-
+    attr_writer :base_url
-    private
-
-    def html_response?(headers)
-      headers['Content-Type'] && headers['Content-Type'] =~ /html/
-    end
-  end
-
-  def self.enabled?
-    SiteSetting.content_security_policy || SiteSetting.content_security_policy_report_only
-  end
-
-  def initialize(request = nil)
-    @request = request
-    @directives = {
-      script_src: script_src,
-      worker_src: [:self, :blob],
-    }
-
-    @directives[:report_uri] = path('/csp_reports') if SiteSetting.content_security_policy_collect_reports
   end
 
   def build
-    policy = ActionDispatch::ContentSecurityPolicy.new
+    builder = Builder.new
 
-    @directives.each do |directive, sources|
+    Extension.theme_extensions.each { |extension| builder << extension }
-      if sources.is_a?(Array)
+    Extension.plugin_extensions.each { |extension| builder << extension }
-        policy.public_send(directive, *sources)
+    builder << Extension.site_setting_extension
-      else
-        policy.public_send(directive, sources)
-      end
-    end
 
-    policy.build
+    builder.build
-  end
-
-  private
-
-  attr_reader :request
-
-  SCRIPT_ASSET_DIRECTORIES = [
-    # [dir, can_use_s3_cdn, can_use_cdn]
-    ['/assets/',             true, true],
-    ['/brotli_asset/',       true, true],
-    ['/extra-locales/',      false, false],
-    ['/highlight-js/',       false, true],
-    ['/javascripts/',        false, true],
-    ['/plugins/',            false, true],
-    ['/theme-javascripts/',  false, true],
-    ['/svg-sprite/',         false, true],
-  ]
-
-  def script_assets(base = base_url, s3_cdn = GlobalSetting.s3_cdn_url, cdn = GlobalSetting.cdn_url)
-    SCRIPT_ASSET_DIRECTORIES.map do |dir, can_use_s3_cdn, can_use_cdn|
-      if can_use_s3_cdn && s3_cdn
-        s3_cdn + dir
-      elsif can_use_cdn && cdn
-        cdn + dir
-      else
-        base + dir
-      end
-    end
-  end
-
-  def script_src
-    sources = [
-      :unsafe_eval,
-      "#{base_url}/logs/",
-      "#{base_url}/sidekiq/",
-      "#{base_url}/mini-profiler-resources/",
-    ]
-
-    sources.concat(script_assets)
-
-    sources << 'https://www.google-analytics.com' if SiteSetting.ga_universal_tracking_code.present?
-    sources << 'https://www.googletagmanager.com' if SiteSetting.gtm_container_id.present?
-
-    sources.concat(SiteSetting.content_security_policy_script_src.split('|'))
-  end
-
-  def base_url
-    @base_url ||= Rails.env.development? ? request.host_with_port : Discourse.base_url
   end
 end
+
+CSP = ContentSecurityPolicy

lib/content_security_policy/builder.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+require_dependency 'content_security_policy/default'
+
+class ContentSecurityPolicy
+  class Builder
+    EXTENDABLE_DIRECTIVES = %i[
+      script_src
+      worker_src
+    ].freeze
+
+    # Make extending these directives no-op, until core includes them in default CSP
+    TO_BE_EXTENDABLE = %i[
+      base_uri
+      connect_src
+      default_src
+      font_src
+      form_action
+      frame_ancestors
+      frame_src
+      img_src
+      manifest_src
+      media_src
+      object_src
+      prefetch_src
+      style_src
+    ].freeze
+
+    def initialize
+      @directives = Default.new.directives
+    end
+
+    def <<(extension)
+      return unless valid_extension?(extension)
+
+      extension.each { |directive, sources| extend_directive(normalize(directive), sources) }
+    end
+
+    def build
+      policy = ActionDispatch::ContentSecurityPolicy.new
+
+      @directives.each do |directive, sources|
+        if sources.is_a?(Array)
+          policy.public_send(directive, *sources)
+        else
+          policy.public_send(directive, sources)
+        end
+      end
+
+      policy.build
+    end
+
+    private
+
+    def normalize(directive)
+      directive.to_s.gsub('-', '_').to_sym
+    end
+
+    def extend_directive(directive, sources)
+      return unless extendable?(directive)
+
+      @directives[directive] ||= []
+
+      if sources.is_a?(Array)
+        @directives[directive].concat(sources)
+      else
+        @directives[directive] << sources
+      end
+    end
+
+    def extendable?(directive)
+      EXTENDABLE_DIRECTIVES.include?(directive)
+    end
+
+    def valid_extension?(extension)
+      extension.is_a?(Hash)
+    end
+  end
+end

lib/content_security_policy/default.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+require_dependency 'content_security_policy'
+
+class ContentSecurityPolicy
+  class Default
+    attr_reader :directives
+
+    def initialize
+      @directives = {}.tap do |directives|
+        directives[:script_src] = script_src
+        directives[:worker_src] = worker_src
+        directives[:report_uri] = report_uri if SiteSetting.content_security_policy_collect_reports
+      end
+    end
+
+    private
+
+    delegate :base_url, to: :ContentSecurityPolicy
+
+    SCRIPT_ASSET_DIRECTORIES = [
+      # [dir, can_use_s3_cdn, can_use_cdn]
+      ['/assets/',             true, true],
+      ['/brotli_asset/',       true, true],
+      ['/extra-locales/',      false, false],
+      ['/highlight-js/',       false, true],
+      ['/javascripts/',        false, true],
+      ['/plugins/',            false, true],
+      ['/theme-javascripts/',  false, true],
+      ['/svg-sprite/',         false, true],
+    ]
+
+    def script_assets(base = base_url, s3_cdn = GlobalSetting.s3_cdn_url, cdn = GlobalSetting.cdn_url)
+      SCRIPT_ASSET_DIRECTORIES.map do |dir, can_use_s3_cdn, can_use_cdn|
+        if can_use_s3_cdn && s3_cdn
+          s3_cdn + dir
+        elsif can_use_cdn && cdn
+          cdn + dir
+        else
+          base + dir
+        end
+      end
+    end
+
+    def script_src
+      [
+        :unsafe_eval,
+        "#{base_url}/logs/",
+        "#{base_url}/sidekiq/",
+        "#{base_url}/mini-profiler-resources/",
+        *script_assets
+      ].tap do |sources|
+        sources << 'https://www.google-analytics.com' if SiteSetting.ga_universal_tracking_code.present?
+        sources << 'https://www.googletagmanager.com' if SiteSetting.gtm_container_id.present?
+      end
+    end
+
+    def worker_src
+      [
+        :self,
+        :blob, # ACE editor registers a service worker with a blob for syntax checking
+      ]
+    end
+
+    def report_uri
+      "#{base_url}/csp_reports"
+    end
+  end
+end

lib/content_security_policy/extension.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+class ContentSecurityPolicy
+  module Extension
+    extend self
+
+    def site_setting_extension
+      { script_src: SiteSetting.content_security_policy_script_src.split('|') }
+    end
+
+    def plugin_extensions
+      [].tap do |extensions|
+        Discourse.plugins.each do |plugin|
+          extensions.concat(plugin.csp_extensions) if plugin.enabled?
+        end
+      end
+    end
+
+    THEME_SETTING = 'extend_content_security_policy'
+
+    def theme_extensions
+      cache['theme_extensions'] ||= find_theme_extensions
+    end
+
+    def clear_theme_extensions_cache!
+      cache['theme_extensions'] = nil
+    end
+
+    private
+
+    def cache
+      @cache ||= DistributedCache.new('csp_extensions')
+    end
+
+    def find_theme_extensions
+      extensions = []
+
+      Theme.find_each do |theme|
+        theme.cached_settings.each do |setting, value|
+          extensions << build_theme_extension(value) if setting.to_s == THEME_SETTING
+        end
+      end
+
+      extensions
+    end
+
+    def build_theme_extension(raw)
+      {}.tap do |extension|
+        raw.split('|').each do |entry|
+          directive, source = entry.split(':', 2).map(&:strip)
+
+          extension[directive] ||= []
+          extension[directive] << source
+        end
+      end
+    end
+  end
+end

lib/content_security_policy/middleware.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+require_dependency 'content_security_policy'
+
+class ContentSecurityPolicy
+  class Middleware
+    def initialize(app)
+      @app = app
+    end
+
+    def call(env)
+      request = Rack::Request.new(env)
+      _, headers, _ = response = @app.call(env)
+
+      return response unless html_response?(headers)
+
+      ContentSecurityPolicy.base_url = request.host_with_port if Rails.env.development?
+
+      headers['Content-Security-Policy'] = policy if SiteSetting.content_security_policy
+      headers['Content-Security-Policy-Report-Only'] = policy if SiteSetting.content_security_policy_report_only
+
+      response
+    end
+
+    private
+
+    delegate :policy, to: :ContentSecurityPolicy
+
+    def html_response?(headers)
+      headers['Content-Type'] && headers['Content-Type'] =~ /html/
+    end
+  end
+end

lib/plugin/instance.rb

@@ -32,7 +32,9 @@ class Plugin::Instance
    :locales,
    :service_workers,
    :styles,
-   :themes].each do |att|
+   :themes,
+   :csp_extensions,
+ ].each do |att|
     class_eval %Q{
       def #{att}
         @#{att} ||= []
@@ -361,6 +363,10 @@ class Plugin::Instance
     DiscoursePluginRegistry.register_svg_icon(icon)
   end
 
+  def extend_content_security_policy(extension)
+    csp_extensions << extension
+  end
+
   # @option opts [String] :name
   # @option opts [String] :nativeName
   # @option opts [String] :fallbackLocale

spec/components/plugin/instance_spec.rb

@@ -10,8 +10,8 @@ describe Plugin::Instance do
   context "find_all" do
     it "can find plugins correctly" do
       plugins = Plugin::Instance.find_all("#{Rails.root}/spec/fixtures/plugins")
-      expect(plugins.count).to eq(2)
+      expect(plugins.count).to eq(3)
-      plugin = plugins[1]
+      plugin = plugins[2]
 
       expect(plugin.name).to eq("plugin-name")
       expect(plugin.path).to eq("#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb")

spec/fixtures/plugins/csp_extension/plugin.rb

@@ -0,0 +1,8 @@
+# name: csp_extension
+# about: Fixture plugin that extends default CSP
+# version: 1.0
+# authors: xrav3nz
+
+extend_content_security_policy(
+  script_src: ['https://from-plugin.com']
+)

spec/lib/content_security_policy/builder_spec.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+describe ContentSecurityPolicy::Builder do
+  let(:builder) { described_class.new }
+
+  describe '#<<' do
+    it 'normalizes directive name' do
+      builder << {
+        script_src: ['symbol_underscore'],
+        'script-src': ['symbol_dash'],
+        'script_src' => ['string_underscore'],
+        'script-src' => ['string_dash'],
+      }
+
+      script_srcs = parse(builder.build)['script-src']
+
+      expect(script_srcs).to include(*%w[symbol_underscore symbol_dash string_underscore symbol_underscore])
+    end
+
+    it 'rejects invalid directives and ones that are not allowed to be extended' do
+      builder << {
+        invalid_src: ['invalid'],
+      }
+
+      expect(builder.build).to_not include('invalid')
+    end
+
+    it 'no-ops on invalid values' do
+      previous = builder.build
+
+      builder << nil
+      builder << 123
+      builder << "string"
+      builder << []
+      builder << {}
+
+      expect(builder.build).to eq(previous)
+    end
+  end
+
+  def parse(csp_string)
+    csp_string.split(';').map do |policy|
+      directive, *sources = policy.split
+      [directive, sources]
+    end.to_h
+  end
+end

spec/lib/content_security_policy_spec.rb

@@ -1,21 +1,24 @@
+# frozen_string_literal: true
 require 'rails_helper'
 
 describe ContentSecurityPolicy do
+  before { ContentSecurityPolicy.base_url = nil }
+
   describe 'report-uri' do
     it 'is enabled by SiteSetting' do
       SiteSetting.content_security_policy_collect_reports = true
-      report_uri = parse(ContentSecurityPolicy.new.build)['report-uri'].first
+      report_uri = parse(policy)['report-uri'].first
-      expect(report_uri).to eq('/csp_reports')
+      expect(report_uri).to eq('http://test.localhost/csp_reports')
 
       SiteSetting.content_security_policy_collect_reports = false
-      report_uri = parse(ContentSecurityPolicy.new.build)['report-uri']
+      report_uri = parse(policy)['report-uri']
       expect(report_uri).to eq(nil)
     end
   end
 
   describe 'worker-src' do
     it 'always has self and blob' do
-      worker_srcs = parse(ContentSecurityPolicy.new.build)['worker-src']
+      worker_srcs = parse(policy)['worker-src']
       expect(worker_srcs).to eq(%w[
         'self'
         blob:
@@ -25,8 +28,8 @@ describe ContentSecurityPolicy do
 
   describe 'script-src' do
     it 'always has self, logster, sidekiq, and assets' do
-      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
+      script_srcs = parse(policy)['script-src']
-      expect(script_srcs).to eq(%w[
+      expect(script_srcs).to include(*%w[
         'unsafe-eval'
         http://test.localhost/logs/
         http://test.localhost/sidekiq/
@@ -46,7 +49,7 @@ describe ContentSecurityPolicy do
       SiteSetting.ga_universal_tracking_code = 'UA-12345678-9'
       SiteSetting.gtm_container_id = 'GTM-ABCDEF'
 
-      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
+      script_srcs = parse(policy)['script-src']
       expect(script_srcs).to include('https://www.google-analytics.com')
       expect(script_srcs).to include('https://www.googletagmanager.com')
     end
@@ -54,7 +57,7 @@ describe ContentSecurityPolicy do
     it 'whitelists CDN assets when integrated' do
       set_cdn_url('https://cdn.com')
 
-      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
+      script_srcs = parse(policy)['script-src']
       expect(script_srcs).to include(*%w[
         https://cdn.com/assets/
         https://cdn.com/brotli_asset/
@@ -67,7 +70,7 @@ describe ContentSecurityPolicy do
 
       global_setting(:s3_cdn_url, 'https://s3-cdn.com')
 
-      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
+      script_srcs = parse(policy)['script-src']
       expect(script_srcs).to include(*%w[
         https://s3-cdn.com/assets/
         https://s3-cdn.com/brotli_asset/
@@ -78,14 +81,57 @@ describe ContentSecurityPolicy do
         http://test.localhost/extra-locales/
       ])
     end
+  end
 
-    it 'can be extended with more sources' do
+  it 'can be extended by plugins' do
-      SiteSetting.content_security_policy_script_src = 'example.com|another.com'
+    plugin = Class.new(Plugin::Instance) do
-      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
+      attr_accessor :enabled
-      expect(script_srcs).to include('example.com')
+      def enabled?; @enabled; end
-      expect(script_srcs).to include('another.com')
+    end.new(nil, "#{Rails.root}/spec/fixtures/plugins/csp_extension/plugin.rb")
-      expect(script_srcs).to include("'unsafe-eval'")
+
-    end
+    plugin.activate!
+    Discourse.plugins << plugin
+
+    plugin.enabled = true
+    expect(parse(policy)['script-src']).to include('https://from-plugin.com')
+
+    plugin.enabled = false
+    expect(parse(policy)['script-src']).to_not include('https://from-plugin.com')
+
+    Discourse.plugins.pop
+  end
+
+  it 'can be extended by themes' do
+    policy # call this first to make sure further actions clear the cache
+
+    theme = Fabricate(:theme)
+    settings = <<~YML
+      extend_content_security_policy:
+        type: list
+        default: 'script-src: from-theme.com'
+    YML
+    theme.set_field(target: :settings, name: :yaml, value: settings)
+    theme.save!
+
+    expect(parse(policy)['script-src']).to include('from-theme.com')
+
+    theme.update_setting(:extend_content_security_policy, "script-src: https://from-theme.net|worker-src: from-theme.com")
+    theme.save!
+
+    expect(parse(policy)['script-src']).to_not include('from-theme.com')
+    expect(parse(policy)['script-src']).to include('https://from-theme.net')
+    expect(parse(policy)['worker-src']).to include('from-theme.com')
+
+    theme.destroy!
+
+    expect(parse(policy)['script-src']).to_not include('https://from-theme.net')
+    expect(parse(policy)['worker-src']).to_not include('from-theme.com')
+  end
+
+  it 'can be extended by site setting' do
+    SiteSetting.content_security_policy_script_src = 'from-site-setting.com|from-site-setting.net'
+
+    expect(parse(policy)['script-src']).to include('from-site-setting.com', 'from-site-setting.net')
   end
 
   def parse(csp_string)
@@ -94,4 +140,8 @@ describe ContentSecurityPolicy do
       [directive, sources]
     end.to_h
   end
+
+  def policy
+    ContentSecurityPolicy.policy
+  end
 end