Merge commit from fork

* Fix CSP directive injection in sandbox / plugin-types / report-to (GHSA-rqq5-2gf9-4w4q)

The 2020 source-list scrub (gsub /[\n;]/) was not applied to three
caller-byte-interpolating builders:

  - build_sandbox_list_directive
  - build_media_type_list_directive  (plugin-types)
  - build_report_to_directive

When any of those caller-supplied values contained a `;` or CR/LF,
the bytes landed verbatim in the Content-Security-Policy header.
Because sandbox and plugin-types are emitted in alphabetical order
before script-src, an injected `; script-src 'unsafe-inline' *`
wins via CSP's first-occurrence rule and disables script-src.

Mirror the existing source-list scrub: replace `;`, `\n`, `\r` with a
space and emit a Kernel.warn (same UX as build_source_list_directive).

Adds three regression specs covering all three builders.

Co-Authored-By: tonghuaroot <tonghuaroot@gmail.com>
Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>

* Apply review feedback: unify source-list scrub, single warn per directive

Three refinements to the GHSA fix:

1. Unify the legacy build_source_list_directive scrub with the new
   scrub_directive_value helper. The new helper's regex is [\n\r;]
   (strict superset of the legacy [\n;]); routing the source-list
   path through it closes the bare-\r smuggling gap on naive
   downstreams without expanding the fix surface.

2. Scrub the joined directive string once instead of per-token, so
   high-cardinality input emits a single Kernel.warn per directive
   rather than N warns. Removes scrub_directive_tokens entirely
   (only caller was the per-token map). Behavior pinned by a new
   spec that asserts exactly one warn for a 3-semicolon input.

3. Move the helper docstring above the method it documents (it was
   above the constant). Reword to reflect the per-directive warn
   contract.

Side effect of (1): two existing source-list deprecation specs
asserted the legacy warn format (contains a ;); the unified helper
uses $~[0].inspect (contains a ";") which renders \n as "\n" in the
warning rather than embedding a literal newline. Updated the two
pre-existing expectations to match — the new format is unambiguously
more readable for humans and the GHSA-fix author already chose
.inspect for the new helper.

Tests: 321 examples, 0 failures (+2 new regression specs: source-list
\r injection, single-warn semantics).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Close test gaps: pin sandbox warn text + add report-only regression spec

Two follow-ups on the review's test-coverage thread:

1. Tightened the sandbox-injection spec from allow(Kernel).to
   receive(:warn) to expect(...).to receive(:warn).with(...).once.
   Pins both halves of the contract: the scrub neutralizes the
   injection AND the deprecation warning fires exactly once per
   offending value. Catches a future regression where someone
   reverts the joined-string scrub to per-token (which would emit
   N warns instead of 1).

2. Added a report-only regression spec. Content-Security-Policy
   builds value through the same #value method regardless of
   :report_only, so the existing fix transparently covers
   Content-Security-Policy-Report-Only — but no spec asserts
   that. A future refactor that split the enforced and
   report-only builders could silently leave Report-Only
   exploitable while the enforced spec stayed green.

Tests: 322 examples, 0 failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>
Co-authored-by: Matt Langlois <fletchto99@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 3 people

lib/secure_headers/headers/content_security_policy.rb

@@ -87,7 +87,7 @@ def build_sandbox_list_directive(directive)
       elsif sandbox_list && sandbox_list.any?
         [
           symbol_to_hyphen_case(directive),
-          sandbox_list.uniq
+          scrub_directive_value(directive, sandbox_list.uniq.join(" "))
         ].join(" ")
       end
     end
@@ -97,15 +97,37 @@ def build_media_type_list_directive(directive)
       if media_type_list && media_type_list.any?
         [
           symbol_to_hyphen_case(directive),
-          media_type_list.uniq
+          scrub_directive_value(directive, media_type_list.uniq.join(" "))
         ].join(" ")
       end
     end
 
     def build_report_to_directive(directive)
       return unless endpoint_name = @config.directive_value(directive)
       if endpoint_name && endpoint_name.is_a?(String) && !endpoint_name.empty?
-        [symbol_to_hyphen_case(directive), endpoint_name].join(" ")
+        [symbol_to_hyphen_case(directive), scrub_directive_value(directive, endpoint_name)].join(" ")
+      end
+    end
+
+    # Bytes that would let a caller-controlled value break out of its
+    # directive and inject sibling CSP directives. CR/LF are included
+    # so naive downstreams that split on bare \r can't be used to
+    # smuggle directives either.
+    DIRECTIVE_INJECTION_REGEX = /[\n\r;]/.freeze
+
+    # Private: scrubs caller-controlled bytes that would let a value
+    # break out of its CSP directive (CR, LF, semicolon). Shared across
+    # every directive builder so sandbox / plugin-types / report-to /
+    # source-list all reject the same byte set with the same warn UX.
+    # Emits a single Kernel.warn per directive even when multiple
+    # offending bytes are present.
+    def scrub_directive_value(directive, value)
+      str = value.to_s
+      if str =~ DIRECTIVE_INJECTION_REGEX
+        Kernel.warn("#{directive} contains a #{$~[0].inspect} in #{str.inspect} which will raise an error in future versions. It has been replaced with a blank space.")
+        str.gsub(DIRECTIVE_INJECTION_REGEX, " ")
+      else
+        str
       end
     end
 
@@ -118,12 +140,7 @@ def build_source_list_directive(directive)
       source_list = @config.directive_value(directive)
       if source_list != OPT_OUT && source_list && source_list.any?
         minified_source_list = minify_source_list(directive, source_list).join(" ")
-
-        if minified_source_list =~ /(\n|;)/
-          Kernel.warn("#{directive} contains a #{$1} in #{minified_source_list.inspect} which will raise an error in future versions. It has been replaced with a blank space.")
-        end
-
-        escaped_source_list = minified_source_list.gsub(/[\n;]/, " ")
+        escaped_source_list = scrub_directive_value(directive, minified_source_list)
         [symbol_to_hyphen_case(directive), escaped_source_list].join(" ").strip
       end
     end

spec/lib/secure_headers/headers/content_security_policy_spec.rb

@@ -29,12 +29,12 @@ module SecureHeaders
       end
 
       it "deprecates and escapes semicolons in directive source lists" do
-        expect(Kernel).to receive(:warn).with(%(frame_ancestors contains a ; in "google.com;script-src *;.;" which will raise an error in future versions. It has been replaced with a blank space.))
+        expect(Kernel).to receive(:warn).with(%(frame_ancestors contains a ";" in "google.com;script-src *;.;" which will raise an error in future versions. It has been replaced with a blank space.))
         expect(ContentSecurityPolicy.new(frame_ancestors: %w(https://google.com;script-src https://*;.;)).value).to eq("frame-ancestors google.com script-src * .")
       end
 
       it "deprecates and escapes semicolons in directive source lists" do
-        expect(Kernel).to receive(:warn).with(%(frame_ancestors contains a \n in "\\nfoo.com\\nhacked" which will raise an error in future versions. It has been replaced with a blank space.))
+        expect(Kernel).to receive(:warn).with(%(frame_ancestors contains a "\\n" in "\\nfoo.com\\nhacked" which will raise an error in future versions. It has been replaced with a blank space.))
         expect(ContentSecurityPolicy.new(frame_ancestors: ["\nfoo.com\nhacked"]).value).to eq("frame-ancestors  foo.com hacked")
       end
 
@@ -243,6 +243,84 @@ module SecureHeaders
         csp = ContentSecurityPolicy.new({ default_src: %w('self'), report_to: "reporting-endpoint-name" })
         expect(csp.value).to eq("default-src 'self'; report-to reporting-endpoint-name")
       end
+
+      it "strips semicolons and newlines from sandbox tokens to prevent directive injection" do
+        # Pins both halves of the contract: scrub neutralizes the
+        # injection AND the deprecation warning fires exactly once
+        # for the offending value (the joined-string scrub means one
+        # warn per directive regardless of token count or how many
+        # offending bytes a token contains).
+        expect(Kernel).to receive(:warn).with(%(sandbox contains a ";" in "allow-forms; script-src 'unsafe-inline' * allow-scripts" which will raise an error in future versions. It has been replaced with a blank space.)).once
+        csp = ContentSecurityPolicy.new(
+          default_src: %w('self'),
+          sandbox: ["allow-forms; script-src 'unsafe-inline' *", "allow-scripts"],
+          script_src: %w('self')
+        )
+        expect(csp.value).not_to match(/sandbox[^;]*;\s*script-src 'unsafe-inline'/)
+        expect(csp.value).to include("sandbox allow-forms  script-src 'unsafe-inline' * allow-scripts")
+      end
+
+      it "strips semicolons and newlines from plugin-types tokens to prevent directive injection" do
+        allow(Kernel).to receive(:warn)
+        csp = ContentSecurityPolicy.new(
+          default_src: %w('self'),
+          plugin_types: ["application/pdf;\nscript-src 'unsafe-inline' *"],
+          script_src: %w('self')
+        )
+        expect(csp.value).not_to match(/plugin-types[^;]*;\s*script-src 'unsafe-inline'/)
+        expect(csp.value).not_to include("\n")
+        expect(csp.value).not_to include("plugin-types application/pdf;")
+      end
+
+      it "strips semicolons and newlines from report-to endpoint to prevent directive injection" do
+        allow(Kernel).to receive(:warn)
+        csp = ContentSecurityPolicy.new(
+          default_src: %w('self'),
+          report_to: "csp-endpoint; script-src 'unsafe-inline' *",
+          script_src: %w('self')
+        )
+        expect(csp.value).not_to match(/report-to[^;]*;\s*script-src 'unsafe-inline'/)
+        expect(csp.value).to include("report-to csp-endpoint  script-src 'unsafe-inline' *")
+      end
+
+      it "strips carriage returns from source-list values to prevent directive injection" do
+        allow(Kernel).to receive(:warn)
+        csp = ContentSecurityPolicy.new(
+          default_src: %w('self'),
+          script_src: ["'self'\rscript-src 'unsafe-inline' *"]
+        )
+        expect(csp.value).not_to include("\r")
+        expect(csp.value).to include("script-src 'self' script-src 'unsafe-inline' *")
+      end
+
+      it "emits a single Kernel.warn per directive even when multiple offending bytes are present" do
+        # Per-directive (not per-token) warn semantics so high-cardinality
+        # input can't spam stderr / error trackers.
+        expect(Kernel).to receive(:warn).once
+        ContentSecurityPolicy.new(
+          default_src: %w('self'),
+          sandbox: ["allow-forms;allow-scripts;allow-popups"]
+        ).value
+      end
+
+      it "applies the directive-injection scrub in report-only mode too" do
+        # Report-only CSP goes through the same `value` builder as
+        # enforced CSP, so the scrub is structurally inherited — but
+        # nothing pins that. A future refactor that split the builders
+        # (e.g., to differentiate report-only semantics) could silently
+        # leave Content-Security-Policy-Report-Only exploitable while
+        # the enforced spec stayed green. Pin it explicitly.
+        allow(Kernel).to receive(:warn)
+        csp = ContentSecurityPolicy.new(
+          default_src: %w('self'),
+          sandbox: ["allow-forms; script-src 'unsafe-inline' *"],
+          script_src: %w('self'),
+          report_only: true
+        )
+        expect(csp.name).to eq(ContentSecurityPolicyReportOnlyConfig::HEADER_NAME)
+        expect(csp.value).not_to match(/sandbox[^;]*;\s*script-src 'unsafe-inline'/)
+        expect(csp.value).to include("sandbox allow-forms  script-src 'unsafe-inline' *")
+      end
     end
   end
 end