FIX: invalid CSP directive sources should allow site to boot with valid CSP directives (stable) (#31270)

[Security
patch](5558e72f22)
(for this [CVE](https://nvd.nist.gov/vuln/detail/CVE-2024-54133)) from
rails actionpack was backported from [Rails
8.0.0.1](https://github.com/rails/rails/blob/v8.0.1/actionpack/CHANGELOG.md#rails-8001-december-10-2024)
to previous stable versions including `7-1-stable` / `7-2-stable`.

Any previous version of Discourse upgrading to v3.4.0.beta3 and above
would have observed their sites crashing if they had invalid sources in
their CSP directive extensions.

This fix removes such invalid sources during our build of the CSP, and
logs these at a warning level so devs are able to find out why their CSP
sources were filtered out of the extendable directives.

lib/content_security_policy/builder.rb

@@ -83,7 +83,18 @@ class ContentSecurityPolicy
           sources.reject { |s| s == "'unsafe-inline'" || s == "'self'" || !s.start_with?("'") }
       end
 
-      @directives[directive].concat(sources)
+      sources.each do |source|
+        # mirrors validation check in ActionDispatch::ContentSecurityPolicy https://github.com/rails/rails/blob/5558e72f22fc69c1c407b31ac5fb3b4ce087b542/actionpack/lib/action_dispatch/http/content_security_policy.rb#L337
+        if source.include?(";") || source != source.gsub(/[[:space:]]/, "")
+          Rails.logger.warn(<<~MSG.squish)
+            Skipping invalid Content Security Policy #{directive}: "#{source}".
+            Directive values must not contain whitespace or semicolons.
+            Please use multiple arguments or other directive methods instead.
+          MSG
+          next
+        end
+        @directives[directive] << source
+      end
 
       @directives[directive].delete(:none) if @directives[directive].count > 1
     end

spec/fixtures/plugins/csp_extension/plugin.rb

@@ -6,7 +6,11 @@
 # authors: xrav3nz
 
 extend_content_security_policy(
-  script_src: %w[https://from-plugin.com /local/path],
+  script_src: [
+    "https://from-plugin.com",
+    "/local/path",
+    "'unsafe-eval' https://invalid.example.com",
+  ],
   object_src: ["https://test-stripping.com"],
   frame_ancestors: ["https://frame-ancestors-plugin.ext"],
   manifest_src: ["https://manifest-src.com"],

spec/lib/content_security_policy/builder_spec.rb

@@ -24,6 +24,13 @@ RSpec.describe ContentSecurityPolicy::Builder do
       expect(builder.build).to_not include("invalid")
     end
 
+    it "skips invalid sources with whitespace or semicolons" do
+      invalid_sources = ["invalid source;", "'unsafe-eval' https://invalid.example.com'"]
+      builder << { script_src: invalid_sources }
+      script_srcs = parse(builder.build)["script-src"]
+      invalid_sources.each { |invalid_source| expect(script_srcs).not_to include(invalid_source) }
+    end
+
     it "no-ops on invalid values" do
       previous = builder.build
 

spec/lib/content_security_policy_spec.rb

@@ -98,6 +98,7 @@ RSpec.describe ContentSecurityPolicy do
     let(:plugin_class) do
       Class.new(Plugin::Instance) do
         attr_accessor :enabled
+
         def enabled?
           @enabled
         end

spec/system/content_security_policy_spec.rb

@@ -1,11 +1,36 @@
 # frozen_string_literal: true
 
 describe "Content security policy", type: :system do
-  it "can boot the application in strict_dynamic mode" do
-    expect(SiteSetting.content_security_policy).to eq(true)
+  let(:plugin_class) do
+    Class.new(Plugin::Instance) do
+      attr_accessor :enabled
 
+      def enabled?
+        @enabled
+      end
+    end
+  end
+
+  it "can boot the application in strict_dynamic mode even with invalid directives from CSP extensions" do
+    plugin = plugin_class.new(nil, "#{Rails.root}/spec/fixtures/plugins/csp_extension/plugin.rb")
+
+    plugin.activate!
+    Discourse.plugins << plugin
+
+    plugin.enabled = true
+
+    expect(SiteSetting.content_security_policy).to eq(true)
     visit "/"
     expect(page).to have_css("#site-logo")
+
+    get "/"
+    expect(response.headers["Content-Security-Policy"]).to include("'strict-dynamic'")
+    expect(response.headers["Content-Security-Policy"]).not_to include(
+      "'unsafe-eval' https://invalid.example.com'",
+    )
+
+    Discourse.plugins.delete plugin
+    DiscoursePluginRegistry.reset!
   end
 
   it "works for 'public exceptions' like RoutingError" do