javascript_include_tag needs to support new nonce: true option

[richhollis]

Rails 5.2 added the nonce: true option to the asset tag helper, which when set should add the CSP nonce to the tag.

Rails: https://github.com/rails/rails/blob/b9ca94caea2ca6a6cc09abaffaad67b447134079/actionview/lib/action_view/helpers/asset_tag_helper.rb#L98-L99

Teaspon: https://github.com/jejacks0n/teaspoon/blob/dbe293dfcf61c1756f61f8696e066c436f7dd57b/lib/teaspoon/engine.rb#L107-L117

My system was just outputting nonce: true. I traced back the reason for this to the teaspoon gem. It overrides the Rails implementation but doesn't include the newer logic.

Should teaspoon have this newer logic or a complete updated copy of the latest version of javascript_include_tag with the addition of the path_options[:debug] option? If so, I would be happy to submit a PR.

[eric-hemasystems]

Workaround until this is fixed. Drop this in your helpers directory:

module TeaspoonHelper
  # Hack until https://github.com/jejacks0n/teaspoon/issues/574 is fixed
  def javascript_include_tag *sources
    options = sources.extract_options!.stringify_keys
    options['nonce'] = content_security_policy_nonce if options['nonce']
    sources << options
    super *sources
  end
end

[mathieujobin]

@eric-hemasystems can your patch be merged ?

[eric-hemasystems]

My hack probably isn't directly suitable for being put in this project. What it does it provides a new helper that then calls your helper (via super) which is still the overwritten version of what was in an old version of Rails.

Ideally it would be nice if your debug option could be added somehow without completely replacing Rail's impl of javascript_include_tag but I cannot really think of a way to do it.

You could copy the latest version of javascript_include_tag from Rails but then you are locking Teaspoon to specific versions of Rails and eventually this will strike again.

Is the allowed_non_precompiled just used internally when running the test suite? If so perhaps you can just have a helper that the test suite loads decorate path_to_javascript? Something like:

def path_to_javascript source, options={}
  options[:debug] = true
  super
end

This way for the duration of the test suite you get your debug flag but the production rails application is using the stock implementation provided by Rails for javascript_include_tag and path_to_javascript.

[mathieujobin]

That makes sense.

Hopefully that method only changed for 5.x and 6.x, meaning that if we drop support for Rails 4.x and below. there would be only two versions to keep.

[mathieujobin]

4.2

      def javascript_include_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!('protocol', 'extname').symbolize_keys
        sources.uniq.map { |source|
          tag_options = {
            "src" => path_to_javascript(source, path_options)
          }.merge!(options)
          content_tag(:script, "", tag_options)
        }.join("\n").html_safe
      end

5.0

      def javascript_include_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!('protocol', 'extname', 'host').symbolize_keys
        sources.uniq.map { |source|
          tag_options = {
            "src" => path_to_javascript(source, path_options)
          }.merge!(options)
          content_tag("script".freeze, "", tag_options)
        }.join("\n").html_safe
      end

5.1

      def javascript_include_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!("protocol", "extname", "host", "skip_pipeline").symbolize_keys
        sources.uniq.map { |source|
          tag_options = {
            "src" => path_to_javascript(source, path_options)
          }.merge!(options)
          content_tag("script".freeze, "", tag_options)
        }.join("\n").html_safe
      end

5.2

      def javascript_include_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!("protocol", "extname", "host", "skip_pipeline").symbolize_keys
        early_hints_links = []

        sources_tags = sources.uniq.map { |source|
          href = path_to_javascript(source, path_options)
          early_hints_links << "<#{href}>; rel=preload; as=script"
          tag_options = {
            "src" => href
          }.merge!(options)
          if tag_options["nonce"] == true
            tag_options["nonce"] = content_security_policy_nonce
          end
          content_tag("script".freeze, "", tag_options)
        }.join("\n").html_safe

        request.send_early_hints("Link" => early_hints_links.join("\n")) if respond_to?(:request) && request

        sources_tags
      end

6.0

      def javascript_include_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!("protocol", "extname", "host", "skip_pipeline").symbolize_keys
        early_hints_links = []

        sources_tags = sources.uniq.map { |source|
          href = path_to_javascript(source, path_options)
          early_hints_links << "<#{href}>; rel=preload; as=script"
          tag_options = {
            "src" => href
          }.merge!(options)
          if tag_options["nonce"] == true
            tag_options["nonce"] = content_security_policy_nonce
          end
          content_tag("script", "", tag_options)
        }.join("\n").html_safe

        request.send_early_hints("Link" => early_hints_links.join("\n")) if respond_to?(:request) && request

        sources_tags
      end

6.1

      def javascript_include_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!("protocol", "extname", "host", "skip_pipeline").symbolize_keys
        preload_links = []
        nopush = options["nopush"].nil? ? true : options.delete("nopush")
        crossorigin = options.delete("crossorigin")
        crossorigin = "anonymous" if crossorigin == true
        integrity = options["integrity"]

        sources_tags = sources.uniq.map { |source|
          href = path_to_javascript(source, path_options)
          if preload_links_header && !options["defer"]
            preload_link = "<#{href}>; rel=preload; as=script"
            preload_link += "; crossorigin=#{crossorigin}" unless crossorigin.nil?
            preload_link += "; integrity=#{integrity}" unless integrity.nil?
            preload_link += "; nopush" if nopush
            preload_links << preload_link
          end
          tag_options = {
            "src" => href,
            "crossorigin" => crossorigin
          }.merge!(options)
          if tag_options["nonce"] == true
            tag_options["nonce"] = content_security_policy_nonce
          end
          content_tag("script", "", tag_options)
        }.join("\n").html_safe

        if preload_links_header
          send_preload_links_header(preload_links)
        end

        sources_tags
      end

[mathieujobin]

@eric-hemasystems I am not even finding where this debug option is used. maybe we should just remove that super legacy monkey patch

looking at the git history it was introduced to fix https://github.com/jejacks0n/teaspoon/issues/443 and this issue contains several other proposal solutions.

I think they monkeypatch should be removed like you said.

This issue has good info as well https://github.com/rails/sprockets-rails/issues/297

initial version in https://github.com/jejacks0n/teaspoon/commit/1e3a22311fc505caf70e20f752a59e19a2eab9d2

[eric-hemasystems]

I looks like it's used by sprockets (what implements the Rails asset pipeline):

https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L140

A slight earlier version of where this was introduced was here:

https://github.com/jejacks0n/teaspoon/pull/318/files

It looks like the goal is when running the test suite to ensure the assets are easier to debug by not doing the concat and compress steps. But I think this could be accomplished by setting the debug_assets param or by configuring the asset pipeline to config.assets.debug for sprockets in the test environment. Seems either of those would be preferable to monkey-patching a Rails method.

[mathieujobin]

@eric-hemasystems I like your suggestions to simply set the config option. I really don't like we have a monkey patch for debugging purposes running in production.

I open this pull request to remove it. https://github.com/jejacks0n/teaspoon/pull/591 which effectively close this issue, as we won't need to update the monkey patch to add the new nonce: true option

[mathieujobin]

This won't be needed as the monkey patch will be removed in v1.2.2