Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to pass nonce attribute to stylesheet_link_tag and javascript_include_tag helpers used in Trestle views #529

Open
francktrouillez opened this issue Dec 2, 2024 · 0 comments

Comments

@francktrouillez
Copy link

francktrouillez commented Dec 2, 2024

Issue

Currently, it is not possible to enforce the javascript_include_tag or stylesheet_link_tag tags used inside the gem to use nonce attribute, used for Content Security Policy (CSP) headers.

For instance, in the app/views/layouts/trestle/admin.html.erb layout, we use many stylesheet_link_tag and javascript_include_tag helpers to include the stylesheets and javascript files respectively. However, as far as I know, there's no built-in way to enforce the use of the nonce attribute in these tags.

<%= stylesheet_link_tag "trestle/admin", 'data-turbo-track': 'reload' %>
<%= stylesheet_link_tag "trestle/icons/font-awesome", 'data-turbo-track': 'reload' if defined?(Sprockets) %>
<%= stylesheet_link_tag "trestle/custom", 'data-turbo-track': 'reload' %>

<%= javascript_include_tag "trestle/admin", 'data-turbo-track': 'reload' %>
<%= javascript_include_tag "trestle/custom", 'data-turbo-track': 'reload' %>

Is there any way to enforce the use of the nonce attribute in these tags? I couldn't find one but I might be missing something.

Proposed solutions

If we don't have a built-in way to enforce the use of the nonce attribute in these tags, I thought of 2 ways to solve this issue:

  • Option 1:
    • Add a configuration option to Trestle to allow users to set whether or not they want to use the nonce attribute in the tags. This would be a simple boolean configuration option that would be set to false by default. It could be use_nonce_for_scripts and use_nonce_for_styles for instance.

    • Inside the gem, we would check these configuration options and add the nonce attribute to the tags if the configuration option is set to true, by setting it manually to all places using the stylesheet_link_tag and javascript_include_tag helpers.

    • For instance, it would turn this:

      <%= javascript_include_tag "trestle/admin", 'data-turbo-track': 'reload' %>
      <%= stylesheet_link_tag "trestle/admin", 'data-turbo-track': 'reload' %>

      into this:

      <%= javascript_include_tag "trestle/admin", { 'data-turbo-track': 'reload', nonce: Trestle.config.use_nonce_for_scripts.presence }.compact %>
      <%= stylesheet_link_tag "trestle/admin", { 'data-turbo-track': 'reload', nonce: Trestle.config.use_nonce_for_styles.presence && content_security_policy_nonce }.compact  %>

      Note 1: I'm using Object#presence with Hash#compact to avoid adding nonce: false to the tags, since use_nonce_for_scripts == false means we don't want nonce to appear.

      Note 2: for stylesheet_link_tag, it isn't as simple as for javascript_include_tag because the option nonce: true was added starting from Rails 7.2, but I see in the gemspec that Trestle supports Rails 6.0+, so we need to explicitly pass the nonce value to the stylesheet_link_tag helper to remain compatible with Rails 6.0+)

    • Main advantage I can identify is that this is a simple solution that would allow the tags used in the gem to use nonce attribute in the tags if needed.

    • Main disadvantage I can identify is that it requires to change the code in the gem to add the nonce attribute to the tags, and to the future ones as well.

  • Option 2:
    • Same as option 1, we add a configuration option to Trestle to allow users to set whether or not they want to use the nonce attribute in the tags. This would be a simple boolean configuration option that would be set to false by default. It could be use_nonce_for_scripts and use_nonce_for_styles for instance.
    • We create a new helper that would override the stylesheet_link_tag and javascript_include_tag helpers to add the nonce attribute if the configuration option is set to true. This is more elegant as it would allow users to keep using the stylesheet_link_tag and javascript_include_tag helpers as they are used to, without having to change anything in their code, but the methods would rely on the current Rails implementation of these helpers, which could be a problem if Rails changes the implementation in the future. I however think that this is unlikely to happen, as the helpers are widely used and changing the implementation would break many applications.
    • For instance, the helper would look like this:
      # app/helpers/trestle/nonce_helper.rb
      
      module Trestle
        # [Internal]
        module NonceHelper
          def javascript_include_tag(*sources, **options)
            options[:nonce] = true if !options.key?(:nonce) && Trestle.config.use_nonce_for_scripts
            super
          end
      
          def stylesheet_link_tag(*sources, **options)
            # The option 'nonce: true' for stylesheet_link_tag is not supported by Rails below 7.2.
            # To ensure retrocompatibility, we directly set the nonce attribute using the content_security_policy_nonce helper.
            options[:nonce] = content_security_policy_nonce if !options.key?(:nonce) && Trestle.config.use_nonce_for_styles
            super
          end
        end
      end
      which would allow to keep using the helpers as they are used to:
      <%= javascript_include_tag "trestle/admin", 'data-turbo-track': 'reload' %>
      <%= stylesheet_link_tag "trestle/admin", 'data-turbo-track': 'reload' %>
    • Main advantage I can identify is that it allows developers to keep using the stylesheet_link_tag and javascript_include_tag helpers as they are used to, without having to worry about nonce.
    • Main disadvantage I can identify is that it relies on the current Rails implementation of these helpers, which could be a problem if Rails changes the implementation in the future.

An alternative solution would be to add a new helper that would be used instead of the stylesheet_link_tag and javascript_include_tag helpers (like stylesheet_link_tag_trestle and javascript_include_tag_trestle), but I think it would be less elegant and more confusing for developers used to those ActionView helpers.

I'd be happy to implement any of these solutions if you think it makes sense. I'm also happy to discuss the idea further if you think it's worth it.

Of course, if I'm missing something and there's already a way to enforce the use of the nonce attribute in the tags, please let me know.

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant