Automatically add a nonce to script tag if a csp-nonce meta tag is available to get it from #101

TylerRick · 2019-09-10T23:02:26Z

Resolves #100

lib/xray/middleware.rb

spec/xray/middleware_spec.rb

spec/dummy/app/controllers/application_controller.rb

mattbrictson · 2019-09-11T16:10:53Z

Thanks for the PR! I left some initial comments. I won't be able to approve or merge this PR until we get Travis CI working.

@brentd or @TylerRick do you know why we aren't seeing CI checks on this PR?

mattbrictson · 2019-09-11T22:59:11Z

I manually restarted the Travis CI build and it seems to be working now 🤔

mattbrictson · 2019-09-12T03:26:17Z

@TylerRick This is failing CI because CI is testing against Rails < 6. I think the appropriate next steps are:

Open a separate PR to add Rails 6 to the CI matrix
Update this PR to only run CSP-related test code for Rails >= 6.0.0

Can you help with either or both of those?

TylerRick · 2021-02-24T09:39:27Z

@TylerRick This is failing CI because CI is testing against Rails < 6. I think the appropriate next steps are:

Open a separate PR to add Rails 6 to the CI matrix

Update this PR to only run CSP-related test code for Rails >= 6.0.0

Good catch. I've updated it to only run CSP-related test code for Rails >= 5.2.

I also added Rails 6 (and 5.2) to the testing matrix. It's a really trivial change. I don't think we need a separate PR for that unless you're likely to not merge this one right away... Also had to fix another issue related to sprockets 4... Everything is in pretty self-contained commits if we need to cherry pick anything.

Now just trying to fix the test failures... hopefully they'll pass this time around 🤞

  3) Xray Bar includes the controller and action
     Failure/Error: find('#xray-bar').should have_text('ApplicationController#root')
     
     Capybara::ElementNotFound:
       Unable to find css "#xray-bar"
     # ./spec/xray/xray_bar_spec.rb:7:in `block (2 levels) in <top (required)>'

Spread out the Rails versions in travis.yml a bit and test 5.2, not just 5.0

Ruby 2.4 is end-of-life (https://www.ruby-lang.org/en/news/2020/04/05/support-of-ruby-2-4-has-ended/)

This resolves a bunch of (6) test failures that were failing due to an error page that said: To allow requests to www.example.com, add the following to your environment configuration: config.hosts << "www.example.com" This was added as part of ActionDispatch::HostAuthorization in Rails 6. See: - https://guides.rubyonrails.org/6_0_release_notes.html#action-pack-notable-changes - https://guides.rubyonrails.org/configuring.html#configuring-middleware

…ometimes get added by Sprockets 4 pipelines

…ng Ctrl+Shift+X makes the Xray UI appear This verifies that there are no JavaScript errors that would prevent it from working, even though the xray.js script was injected into the document (as tested by spec/xray/middleware_spec.rb).

… test fails

…use some optional debugging tools like byebug and pry

(Since Gemfile.lock isn't currently under version control, and probably can't be since RAILS_VERSION is dynamic.) Without this, it will install the latest version, 4.0, which uses &. not available in older Rubies, causing errors like this: ruby/2.1.0/gems/rspec-rails-4.0.0/lib/rspec/rails/example/mailer_example_group.rb:25: syntax error, unexpected '.' options&.each { |key, value| default_u...

Sprockets::Railtie::ManifestNeededError: Expected to find a manifest file in `app/assets/config/manifest.js` but did not

… puts page.html instead so we can figure out why it failed

... if a csp-nonce meta tag is available to get it from. - Added test that confirms that nonce is added - Added an end-to-end test that uses a strict CSP. Confirmed that this fails without the nonce added to javascript_include_tag calls. - With Sprockets 4, it doesn't add a separate script tag for jquery, so add an explicit javascript_include_tag "jquery" for that case Co-authored-by: Matt Brictson <mattbrictson@users.noreply.github.com>

TylerRick · 2021-02-24T19:25:30Z

Okay, finally got tests passing. Merge please?

Wasted too much time getting it to work with old Rubies/Rails, when I think the correct path should be to just remove support for them going forward (#109)...

Also, now that I realize how easy it is to just manually include this in my app:

<%= javascript_include_tag 'xray', nonce: true if Rails.env.development? %>

I regret even wasting my time getting it to automatically look for and add a nonce to the auto-injected xray.js script. I think we should even consider just removing the auto-injecting feature (#110).

But in any case, even if that ends up getting removed in the future, this PR still has some changes that will be valuable going forward, such as updates to support Sprockets 4, and a simple end-to-end test to make sure the JS actually works (and there weren't any run-time JS errors)...

rocket-turtle · 2021-10-15T05:33:47Z

Hi @mattbrictson,

I love this gem and wie just upgraded to sprockets 4. #113
If this MR had been merged it would have saved us a lot of time and trouble.

Do you plan to continue merging MR and releaseing new versions for this Gem?
I don't think there is any similar gem like this, so it would be sad to see it fade away.

If I can help in any way please let me know.

rocket-turtle · 2024-12-20T16:12:57Z

Hi @brentd,

I just saw that you released a new version for the gem.
Do you plan to release more versions?
Are you willing to accept pull requests?

I would like to contribute to this gem.

Currentle I'm working on my own branch which is based on this Pull request.

If you merge this in I can add some value:

cc3f449 - drop support for Ruby 1.9 - remove xray/aliasing (1 year ago)
bf96eea - use rspec-rails 6.1 (1 year ago)
e0b837d - use and test rails 6.1 (1 year ago)
f5966e1 - fix spec for macs (1 year ago)
0d0ed27 - Convert specs to RSpec 3.9.3 syntax with Transpec (1 year ago)
e293c7c - FIX ArgumentError: wrong number of arguments (given 4, expected 3) (1 year ago)
e536b07 - remove non working capybara-screenshot (1 year ago)

Hope to help
Jonas

brentd · 2024-12-30T20:46:16Z

Hi @rocket-turtle - since I'm rewriting xray, in the meantime I will accept PRs with small non-breaking changes. From what I can tell your changes seem OK; I can probably make an exception for removing Ruby 1.9 support since your change toward Module prepend is good.

However, I may end up removing any/ all contributions in the rewrite, so please be aware. Once released I will tag the rewrite version 1.0 and as it will likely not support old versions of Ruby/Rails, pre-1.0 users may still benefit from your changes.

mattbrictson reviewed Sep 11, 2019

View reviewed changes

TylerRick force-pushed the add_nonce branch 4 times, most recently from e2df4c9 to 7c6bcf8 Compare February 24, 2021 09:37

TylerRick force-pushed the add_nonce branch 2 times, most recently from 246c711 to fb29639 Compare February 24, 2021 18:28

TylerRick mentioned this pull request Feb 24, 2021

Drop support for Ruby <=2.4 (and Rails 4?) #109

Open

TylerRick and others added 12 commits February 24, 2021 10:56

Test on Rails 6

31daaee

Spread out the Rails versions in travis.yml a bit and test 5.2, not just 5.0

Don't test so many old versions

17c7148

Ruby 2.4 is end-of-life (https://www.ruby-lang.org/en/news/2020/04/05/support-of-ruby-2-4-has-ended/)

Update script_matcher to allow for .source and .debug suffixes that s…

c0558bc

…ometimes get added by Sprockets 4 pipelines

Add capybara-screenshot so we can check out the .html document when a…

8d3f195

… test fails

Load gems from Gemfile.local too, if present, so that developers can …

6fed81f

…use some optional debugging tools like byebug and pry

Fix test error with Rails 6:

1f1555c

Sprockets::Railtie::ManifestNeededError: Expected to find a manifest file in `app/assets/config/manifest.js` but did not

Fix test failures on Rails < 6 due to sprockets >= 4

b9efad2

In CI, we can't open the .html files saved by capybara-screenshot, so…

44a878e

… puts page.html instead so we can figure out why it failed

TylerRick force-pushed the add_nonce branch from fb29639 to 63c0210 Compare February 24, 2021 18:57

TylerRick mentioned this pull request Feb 24, 2021

Remove auto-adding after jquery.js script — Sprockets 4 (nor webpack-based javascript_pack_tag 'application') doesn't add a separate script tag for jquery #110

Open

rocket-turtle mentioned this pull request Oct 15, 2021

xray-rails 0.3.2 and sprockets 4 #113

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically add a nonce to script tag if a csp-nonce meta tag is available to get it from #101

Automatically add a nonce to script tag if a csp-nonce meta tag is available to get it from #101

TylerRick commented Sep 10, 2019

mattbrictson commented Sep 11, 2019 •

edited

Loading

mattbrictson commented Sep 11, 2019

mattbrictson commented Sep 12, 2019

TylerRick commented Feb 24, 2021

TylerRick commented Feb 24, 2021 •

edited

Loading

rocket-turtle commented Oct 15, 2021

rocket-turtle commented Dec 20, 2024

brentd commented Dec 30, 2024 •

edited

Loading

Automatically add a nonce to script tag if a csp-nonce meta tag is available to get it from #101

Are you sure you want to change the base?

Automatically add a nonce to script tag if a csp-nonce meta tag is available to get it from #101

Conversation

TylerRick commented Sep 10, 2019

mattbrictson commented Sep 11, 2019 • edited Loading

mattbrictson commented Sep 11, 2019

mattbrictson commented Sep 12, 2019

TylerRick commented Feb 24, 2021

TylerRick commented Feb 24, 2021 • edited Loading

rocket-turtle commented Oct 15, 2021

rocket-turtle commented Dec 20, 2024

brentd commented Dec 30, 2024 • edited Loading

mattbrictson commented Sep 11, 2019 •

edited

Loading

TylerRick commented Feb 24, 2021 •

edited

Loading

brentd commented Dec 30, 2024 •

edited

Loading