From 724d2e99de9ed0d5331eb67458e2ab3f0d978d98 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 2 Apr 2020 11:16:38 -0400
Subject: [PATCH] DEV: Only include "report-sample" CSP directive when
 reporting is enabled (#9337)

---
 app/controllers/csp_reports_controller.rb    | 2 +-
 lib/content_security_policy/default.rb       | 2 +-
 spec/lib/content_security_policy_spec.rb     | 7 ++++++-
 spec/requests/csp_reports_controller_spec.rb | 4 ++--
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/app/controllers/csp_reports_controller.rb b/app/controllers/csp_reports_controller.rb
index b0b10912705..30824e3536d 100644
--- a/app/controllers/csp_reports_controller.rb
+++ b/app/controllers/csp_reports_controller.rb
@@ -6,7 +6,7 @@ class CspReportsController < ApplicationController
     raise Discourse::NotFound unless report_collection_enabled?
 
     Logster.add_to_env(request.env, 'CSP Report', report)
-    Rails.logger.warn("CSP Violation: '#{report['blocked-uri']}'")
+    Rails.logger.warn("CSP Violation: '#{report['blocked-uri']}' \n\n#{report['script-sample']}")
 
     head :ok
   end
diff --git a/lib/content_security_policy/default.rb b/lib/content_security_policy/default.rb
index d2f92f20b57..78ca564815c 100644
--- a/lib/content_security_policy/default.rb
+++ b/lib/content_security_policy/default.rb
@@ -48,12 +48,12 @@ class ContentSecurityPolicy
 
     def script_src
       [
-        :report_sample,
         "#{base_url}/logs/",
         "#{base_url}/sidekiq/",
         "#{base_url}/mini-profiler-resources/",
         *script_assets
       ].tap do |sources|
+        sources << :report_sample if SiteSetting.content_security_policy_collect_reports
         sources << :unsafe_eval if Rails.env.development? # TODO remove this once we have proper source maps in dev
         sources << 'https://www.google-analytics.com/analytics.js' if SiteSetting.ga_universal_tracking_code.present?
         sources << 'https://www.googletagmanager.com/gtm.js' if SiteSetting.gtm_container_id.present?
diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb
index fcbf5ece2c7..68a5dc2da2b 100644
--- a/spec/lib/content_security_policy_spec.rb
+++ b/spec/lib/content_security_policy_spec.rb
@@ -46,7 +46,6 @@ describe ContentSecurityPolicy do
     it 'always has self, logster, sidekiq, and assets' do
       script_srcs = parse(policy)['script-src']
       expect(script_srcs).to include(*%w[
-        'report-sample'
         http://test.localhost/logs/
         http://test.localhost/sidekiq/
         http://test.localhost/mini-profiler-resources/
@@ -61,6 +60,12 @@ describe ContentSecurityPolicy do
       ])
     end
 
+    it 'includes "report-sample" when report collection is enabled' do
+      SiteSetting.content_security_policy_collect_reports = true
+      script_srcs = parse(policy)['script-src']
+      expect(script_srcs).to include("'report-sample'")
+    end
+
     it 'whitelists Google Analytics and Tag Manager when integrated' do
       SiteSetting.ga_universal_tracking_code = 'UA-12345678-9'
       SiteSetting.gtm_container_id = 'GTM-ABCDEF'
diff --git a/spec/requests/csp_reports_controller_spec.rb b/spec/requests/csp_reports_controller_spec.rb
index 2c3bfe521da..07e8aeafb46 100644
--- a/spec/requests/csp_reports_controller_spec.rb
+++ b/spec/requests/csp_reports_controller_spec.rb
@@ -29,7 +29,7 @@ describe CspReportsController do
           "line-number": 25,
           "source-file": "http://localhost:3000/",
           "status-code": 200,
-          "script-sample": ""
+          "script-sample": "console.log('unsafe')"
         }
       }.to_json, headers: { "Content-Type": "application/csp-report" }
     end
@@ -52,7 +52,7 @@ describe CspReportsController do
 
     it 'logs the violation report' do
       send_report
-      expect(Rails.logger.warnings).to include("CSP Violation: 'http://suspicio.us/assets.js'")
+      expect(Rails.logger.warnings).to include("CSP Violation: 'http://suspicio.us/assets.js' \n\nconsole.log('unsafe')")
     end
   end
 end