Output only one nonce in CSP header per request

3 files changed, 59 insertions, 22 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 152ec3700b..c85add22a9 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Output only one Content-Security-Policy nonce header value per request.
+
+    Fixes [#35297](https://github.com/rails/rails/issues/32597).
+
+    *Andrey Novikov*
+
 *   Move default headers configuration into their own module that can be included in controllers.
 
     *Kevin Deisz*
diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb
index c1f80a1ffc..0573f13f94 100644
--- a/actionpack/lib/action_dispatch/http/content_security_policy.rb
+++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb
@@ -21,13 +21,7 @@ module ActionDispatch #:nodoc:
         return response if policy_present?(headers)
 
         if policy = request.content_security_policy
-          if policy.directives["script-src"]
-            if nonce = request.content_security_policy_nonce
-              policy.directives["script-src"] << "'nonce-#{nonce}'"
-            end
-          end
-
-          headers[header_name(request)] = policy.build(request.controller_instance)
+          headers[header_name(request)] = policy.build(request)
         end
 
         response
@@ -101,6 +95,14 @@ module ActionDispatch #:nodoc:
         end
     end
 
+    class NonceGenerator
+      def call(request)
+        if nonce = request&.content_security_policy_nonce
+          "'nonce-#{nonce}'"
+        end
+      end
+    end
+
     MAPPINGS = {
       self:           "'self'",
       unsafe_eval:    "'unsafe-eval'",
@@ -131,7 +133,7 @@ module ActionDispatch #:nodoc:
       manifest_src:    "manifest-src",
       media_src:       "media-src",
       object_src:      "object-src",
-      script_src:      "script-src",
+      # script_src handled differently
       style_src:       "style-src",
       worker_src:      "worker-src"
     }.freeze
@@ -159,6 +161,15 @@ module ActionDispatch #:nodoc:
       end
     end
 
+    def script_src(*sources)
+      if sources.first
+        @directives["script-src"] = apply_mappings(sources)
+        @directives["script-src"] << NonceGenerator.new
+      else
+        @directives.delete("script-src")
+      end
+    end
+
     def block_all_mixed_content(enabled = true)
       if enabled
         @directives["block-all-mixed-content"] = true
@@ -205,8 +216,8 @@ module ActionDispatch #:nodoc:
       end
     end
 
-    def build(context = nil)
-      build_directives(context).compact.join("; ")
+    def build(request = nil)
+      build_directives(request).compact.join("; ")
     end
 
     private
@@ -229,10 +240,10 @@ module ActionDispatch #:nodoc:
         end
       end
 
-      def build_directives(context)
+      def build_directives(request)
         @directives.map do |directive, sources|
           if sources.is_a?(Array)
-            "#{directive} #{build_directive(sources, context).join(' ')}"
+            "#{directive} #{build_directive(sources, request).compact.join(' ')}"
           elsif sources
             directive
           else
@@ -241,22 +252,24 @@ module ActionDispatch #:nodoc:
         end
       end
 
-      def build_directive(sources, context)
-        sources.map { |source| resolve_source(source, context) }
+      def build_directive(sources, request)
+        sources.map { |source| resolve_source(source, request) }
       end
 
-      def resolve_source(source, context)
+      def resolve_source(source, request)
         case source
         when String
           source
         when Symbol
           source.to_s
         when Proc
-          if context.nil?
+          if request&.controller_instance.nil?
             raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}"
           else
-            context.instance_exec(&source)
+            request.controller_instance.instance_exec(&source)
           end
+        when NonceGenerator
+          source.call(request)
         else
           raise RuntimeError, "Unexpected content security policy source: #{source.inspect}"
         end
diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb
index 95fce39dad..eb0b930828 100644
--- a/actionpack/test/dispatch/content_security_policy_test.rb
+++ b/actionpack/test/dispatch/content_security_policy_test.rb
@@ -200,16 +200,18 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase
   end
 
   def test_dynamic_directives
-    request = Struct.new(:host).new("www.example.com")
-    controller = Struct.new(:request).new(request)
+    request = ActionDispatch::Request.new("HTTP_HOST" => "www.example.com")
+    request.controller_instance = Struct.new(:request).new(request)
 
     @policy.script_src -> { request.host }
-    assert_equal "script-src www.example.com", @policy.build(controller)
+    assert_equal "script-src www.example.com", @policy.build(request)
   end
 
   def test_mixed_static_and_dynamic_directives
     @policy.script_src :self, -> { "foo.com" }, "bar.com"
-    assert_equal "script-src 'self' foo.com bar.com", @policy.build(Object.new)
+    request = ActionDispatch::Request.new({})
+    request.controller_instance = Struct.new(:request).new(request)
+    assert_equal "script-src 'self' foo.com bar.com", @policy.build(request)
   end
 
   def test_invalid_directive_source
@@ -245,17 +247,21 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest
   class PolicyController < ActionController::Base
     content_security_policy only: :inline do |p|
       p.default_src "https://example.com"
+      p.script_src false
     end
 
     content_security_policy only: :conditional, if: :condition? do |p|
       p.default_src "https://true.example.com"
+      p.script_src false
     end
 
     content_security_policy only: :conditional, unless: :condition? do |p|
       p.default_src "https://false.example.com"
+      p.script_src false
     end
 
     content_security_policy only: :report_only do |p|
+      p.script_src false
       p.report_uri "/violations"
     end
 
@@ -292,6 +298,10 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest
       head :ok
     end
 
+    def default_script_src
+      head :ok
+    end
+
     private
       def condition?
         params[:condition] == "true"
@@ -307,11 +317,13 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest
       get "/report-only", to: "policy#report_only"
       get "/script-src", to: "policy#script_src"
       get "/no-policy", to: "policy#no_policy"
+      get "/default-script-src", to: "policy#default_script_src"
     end
   end
 
   POLICY = ActionDispatch::ContentSecurityPolicy.new do |p|
     p.default_src :self
+    p.script_src  :https
   end
 
   class PolicyConfigMiddleware
@@ -340,7 +352,7 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest
 
   def test_generates_content_security_policy_header
     get "/"
-    assert_policy "default-src 'self'"
+    assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='"
   end
 
   def test_generates_inline_content_security_policy
@@ -366,6 +378,12 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest
     assert_policy "script-src 'self' 'nonce-iyhD0Yc0W+c='"
   end
 
+  def test_adds_nonce_to_script_src_content_security_policy_only_once
+    get "/default-script-src"
+    get "/default-script-src"
+    assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='"
+  end
+
   def test_generates_no_content_security_policy
     get "/no-policy"