Merge branch 'nonce-all-script-tags' into 'master'

Force a nonce on all script tags when CSP is enabled

See merge request gitlab-org/gitlab!48093

app/helpers/defer_script_tag_helper.rb → app/helpers/gitlab_script_tag_helper.rb

 # frozen_string_literal: true
 
-module DeferScriptTagHelper
+module GitlabScriptTagHelper
   # Override the default ActionView `javascript_include_tag` helper to support page specific deferred loading.
   # PLEASE NOTE: `defer` is also critical so that we don't run JavaScript entrypoints before the DOM is ready.
   # Please see https://gitlab.com/groups/gitlab-org/-/epics/4538#note_432159769.
+  # The helper also makes sure the `nonce` attribute is included in every script when the content security
+  # policy is enabled.
   def javascript_include_tag(*sources)
-    super(*sources, defer: true)
+    super(*sources, defer: true, nonce: true)
+  end
+
+  # The helper makes sure the `nonce` attribute is included in every script when the content security
+  # policy is enabled.
+  def javascript_tag(content_or_options_with_block = nil, html_options = {})
+    if content_or_options_with_block.is_a?(Hash)
+      content_or_options_with_block[:nonce] = true
+    else
+      html_options[:nonce] = true
+    end
+
+    super
   end
 end

app/views/layouts/_google_analytics.html.haml

-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     var _gaq = _gaq || [];
     _gaq.push(['_setAccount', '#{extra_config.google_analytics_id}']);

app/views/layouts/_google_tag_manager_head.html.haml

 - if google_tag_manager_enabled?
-  = javascript_tag nonce: true do
+  = javascript_tag do
     :plain
       (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
       new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0],

app/views/layouts/_img_loader.html.haml

-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     if ('loading' in HTMLImageElement.prototype) {
       document.querySelectorAll('img.lazy').forEach(img => {

app/views/layouts/_init_auto_complete.html.haml

@@ -4,7 +4,7 @@
 - datasources = autocomplete_data_sources(object, noteable_type)
 
 - if object
-  = javascript_tag nonce: true do
+  = javascript_tag do
     :plain
       gl = window.gl || {};
       gl.GfmAutoComplete = gl.GfmAutoComplete || {};

app/views/layouts/_init_client_detection_flags.html.haml

 - client = client_js_flags
 
 - if client
-  = javascript_tag nonce: true do
+  = javascript_tag do
     :plain
       gl = window.gl || {};
       gl.client = #{client.to_json};

app/views/layouts/_matomo.html.haml

 <!-- Matomo -->
-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     var _paq = window._paq = window._paq || [];
     _paq.push(['trackPageView']);

app/views/layouts/_snowplow.html.haml

 - return unless Gitlab::CurrentSettings.snowplow_enabled?
 
-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     ;(function(p,l,o,w,i,n,g){if(!p[i]){p.GlobalSnowplowNamespace=p.GlobalSnowplowNamespace||[];
     p.GlobalSnowplowNamespace.push(i);p[i]=function(){(p[i].q=p[i].q||[]).push(arguments)

app/views/layouts/_startup_css_activation.haml

 - return unless use_startup_css?
 
-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     document.querySelectorAll('link[media="print"]').forEach(linkTag => {
       linkTag.setAttribute('data-startupcss', 'loading');

app/views/layouts/_startup_js.html.haml

 - return unless page_startup_api_calls.present? || page_startup_graphql_calls.present?
 
-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     var gl = window.gl || {};
     gl.startup_calls = #{page_startup_api_calls.to_json};

app/views/layouts/errors.html.haml

@@ -8,7 +8,7 @@
   %body
     .page-container
       = yield
-    = javascript_tag nonce: true do
+    = javascript_tag do
       :plain
         (function(){
           var goBackElement = document.querySelector('.js-go-back');

app/views/layouts/group.html.haml

@@ -8,7 +8,7 @@
 
 - content_for :page_specific_javascripts do
   - if current_user
-    = javascript_tag nonce: true do
+    = javascript_tag do
       :plain
         window.uploads_path = "#{group_uploads_path(@group)}";
 

app/views/layouts/project.html.haml

@@ -10,7 +10,7 @@
 - content_for :project_javascripts do
   - project = @target_project || @project
   - if current_user
-    = javascript_tag nonce: true do
+    = javascript_tag do
       :plain
         window.uploads_path = "#{project_uploads_path(project)}";
 

app/views/layouts/snippets.html.haml

@@ -4,7 +4,7 @@
 
 - content_for :page_specific_javascripts do
   - if snippets_upload_path
-    = javascript_tag nonce: true do
+    = javascript_tag do
       :plain
         window.uploads_path = "#{snippets_upload_path}";
 

app/views/projects/merge_requests/_widget.html.haml

-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     window.gl = window.gl || {};
     window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget', issues_links: true)}

ee/app/views/projects/merge_requests/show.html.haml

 = render_ce "projects/merge_requests/show"
 
-= javascript_tag nonce: true do
+= javascript_tag do
   :plain
     // Append static, server-generated data not included in merge request entity (EE-Only)
     // Object.assign would be useful here, but it blows up Phantom.js in tests

spec/helpers/defer_script_tag_helper_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe DeferScriptTagHelper do
-  describe 'script tag' do
-    script_url = 'test.js'
-
-    it 'returns an script tag with defer=true' do
-      expect(javascript_include_tag(script_url).to_s)
-        .to eq "<script src=\"/javascripts/#{script_url}\" defer=\"defer\"></script>"
-    end
-  end
-end

spec/helpers/gitlab_script_tag_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabScriptTagHelper do
+  before do
+    allow(helper).to receive(:content_security_policy_nonce).and_return('noncevalue')
+  end
+
+  describe 'external script tag' do
+    let(:script_url) { 'test.js' }
+
+    it 'returns a script tag with defer=true and a nonce' do
+      expect(helper.javascript_include_tag(script_url).to_s)
+        .to eq "<script src=\"/javascripts/#{script_url}\" defer=\"defer\" nonce=\"noncevalue\"></script>"
+    end
+  end
+
+  describe 'inline script tag' do
+    let(:tag_with_nonce) {"<script nonce=\"noncevalue\">\n//<![CDATA[\nalert(1)\n//]]>\n</script>"}
+    let(:tag_with_nonce_and_type) {"<script type=\"application/javascript\" nonce=\"noncevalue\">\n//<![CDATA[\nalert(1)\n//]]>\n</script>"}
+
+    it 'returns a script tag with a nonce using block syntax' do
+      expect(helper.javascript_tag { 'alert(1)' }.to_s).to eq tag_with_nonce
+    end
+
+    it 'returns a script tag with a nonce using block syntax with options' do
+      expect(helper.javascript_tag(type: 'application/javascript') { 'alert(1)' }.to_s).to eq tag_with_nonce_and_type
+    end
+
+    it 'returns a script tag with a nonce using argument syntax' do
+      expect(helper.javascript_tag('alert(1)').to_s).to eq tag_with_nonce
+    end
+
+    it 'returns a script tag with a nonce using argument syntax with options' do
+      expect(helper.javascript_tag( 'alert(1)', type: 'application/javascript').to_s).to eq tag_with_nonce_and_type
+    end
+
+    # This scenario does not really make sense, but it's supported so we test it
+    it 'returns a script tag with a nonce using argument and block syntax with options' do
+      expect(helper.javascript_tag( '// ignored', type: 'application/javascript') { 'alert(1)' }.to_s).to eq tag_with_nonce_and_type
+    end
+  end
+end