-
Notifications
You must be signed in to change notification settings - Fork 242
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
Move inline JS to rendered files to support CSP #561
base: master
Are you sure you want to change the base?
Conversation
cc @jejacks0n |
This is awesome, thanks @worldwise001! I'm in the process of getting this project up to speed and more current, and I'd like to work with you on getting this in there. My plan is specifically to focus around CSP settings, webpack(er), and reducing the asset pipeline dependency in that process. Maybe you can help -- I'm seeing calls to |
Yes, it should all be already in the project. I just moved some files around so that they are served via the controller instead of being inline JS. |
Seems to be! answered my own question. haha. Ok, so one thing to note here is that right now not all specs / features are being run on CI since I'm still working on getting things polished up. When I get CI back to a place I'm comfortable with can I hit you up to merge master and we can check in at that point? |
(fwiw I tested this using my own project so I can verify that this causes the CSP errors to go away... I haven't really tested require.js since I don't use that in my project). |
Sure! Let me know when you're likely going to be able to get CI in a nice place and I can rebase and retest. |
Thanks so much @worldwise001 -- I appreciate your time and will reach out in the next couple days as time permits. =) |
Assume sauce! Kudos to you both, and sounds like the start of a lovely collaboration. |
1d95ed2
to
d6352cb
Compare
@worldwise001 -- The test suite is running fully and coverage reports are back in there now. There's a flakey test about code coverage, and I haven't resolved that because it's pretty complex -- and seems like a caching/random spec order issue that manifests somewhere inside sprockets. You're welcome to rebase master now, and just keep an eye out for that one test. You can just re-run the failing build(s) until it passes -- sorry about that! Thanks! |
Fantastic! I'll do that today! (sorry been swamped with other unrelated work). |
a1b53af
to
5c5807b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Unable to find gem rubocop-rails_config; is the gem installed? Gem::MissingSp...
Unable to find gem rubocop-rails_config; is the gem installed? Gem::MissingSpecError /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:189:in `rescue in gem_config_path' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:185:in `gem_config_path' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:47:in `block (2 levels) in resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:45:in `each' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:45:in `block in resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:38:in `each_pair' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:38:in `resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader.rb:48:in `load_file' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader.rb:83:in `configuration_from_file' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_store.rb:44:in `for' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/cli.rb:181:in `apply_default_formatter' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/cli.rb:40:in `run' /home/linters/.bundle/gems/rubocop-0.54.0/bin/rubocop:13:in `block in ' /usr/local/lib/ruby/2.6.0/benchmark.rb:308:in `realtime' /home/linters/.bundle/gems/rubocop-0.54.0/bin/rubocop:12:in `' /home/linters/.bundle/bin/rubocop:23:in `load' /home/linters/.bundle/bin/rubocop:23:in `' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `load' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `kernel_load' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:28:in `run' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:463:in `exec' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor.rb:387:in `dispatch' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:27:in `dispatch' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/base.rb:466:in `start' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:18:in `start' /usr/local/lib/ruby/gems/2.6.0/gems/bundler-1.17.2/exe/bundle:30:in `block in ' /usr/local/lib/ruby/2.6.0/bundler/friendly_errors.rb:124:in `with_friendly_errors' /usr/local/lib/ruby/gems/2.6.0/gems/bundler-1.17.2/exe/bundle:22:in `' /usr/local/bin/bundle:23:in `load' /usr/local/bin/bundle:23:in `'
5c5807b
to
7e7b147
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Unable to find gem rubocop-rails_config; is the gem installed? Gem::MissingSp...
Unable to find gem rubocop-rails_config; is the gem installed? Gem::MissingSpecError /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:189:in `rescue in gem_config_path' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:185:in `gem_config_path' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:47:in `block (2 levels) in resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:45:in `each' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:45:in `block in resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:38:in `each_pair' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:38:in `resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader.rb:48:in `load_file' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader.rb:83:in `configuration_from_file' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_store.rb:44:in `for' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/cli.rb:181:in `apply_default_formatter' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/cli.rb:40:in `run' /home/linters/.bundle/gems/rubocop-0.54.0/bin/rubocop:13:in `block in ' /usr/local/lib/ruby/2.6.0/benchmark.rb:308:in `realtime' /home/linters/.bundle/gems/rubocop-0.54.0/bin/rubocop:12:in `' /home/linters/.bundle/bin/rubocop:23:in `load' /home/linters/.bundle/bin/rubocop:23:in `' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `load' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `kernel_load' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:28:in `run' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:463:in `exec' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor.rb:387:in `dispatch' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:27:in `dispatch' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/base.rb:466:in `start' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:18:in `start' /usr/local/lib/ruby/gems/2.6.0/gems/bundler-1.17.2/exe/bundle:30:in `block in ' /usr/local/lib/ruby/2.6.0/bundler/friendly_errors.rb:124:in `with_friendly_errors' /usr/local/lib/ruby/gems/2.6.0/gems/bundler-1.17.2/exe/bundle:22:in `' /usr/local/bin/bundle:23:in `load' /usr/local/bin/bundle:23:in `'
Hrm it looks like something is rewriting all the routes in the test to add /relative in front of it. I imagine this is to test something, but seems to break all the teaspoon routes. Is this intended? I think this is only supposed to kick in for assets? |
(Any hints on where to look? I tried adding binding.pry to the erbs and those seem to be generating properly, but I couldn't figure out where in the stack frame the rewrite was occurring) |
7e7b147
to
b7d7d01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Unable to find gem rubocop-rails_config; is the gem installed? Gem::MissingSp...
Unable to find gem rubocop-rails_config; is the gem installed? Gem::MissingSpecError /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:189:in `rescue in gem_config_path' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:185:in `gem_config_path' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:47:in `block (2 levels) in resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:45:in `each' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:45:in `block in resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:38:in `each_pair' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader_resolver.rb:38:in `resolve_inheritance_from_gems' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader.rb:48:in `load_file' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_loader.rb:83:in `configuration_from_file' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/config_store.rb:44:in `for' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/cli.rb:181:in `apply_default_formatter' /home/linters/.bundle/gems/rubocop-0.54.0/lib/rubocop/cli.rb:40:in `run' /home/linters/.bundle/gems/rubocop-0.54.0/bin/rubocop:13:in `block in ' /usr/local/lib/ruby/2.6.0/benchmark.rb:308:in `realtime' /home/linters/.bundle/gems/rubocop-0.54.0/bin/rubocop:12:in `' /home/linters/.bundle/bin/rubocop:23:in `load' /home/linters/.bundle/bin/rubocop:23:in `' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `load' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `kernel_load' /usr/local/lib/ruby/2.6.0/bundler/cli/exec.rb:28:in `run' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:463:in `exec' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor.rb:387:in `dispatch' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:27:in `dispatch' /usr/local/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/base.rb:466:in `start' /usr/local/lib/ruby/2.6.0/bundler/cli.rb:18:in `start' /usr/local/lib/ruby/gems/2.6.0/gems/bundler-1.17.2/exe/bundle:30:in `block in ' /usr/local/lib/ruby/2.6.0/bundler/friendly_errors.rb:124:in `with_friendly_errors' /usr/local/lib/ruby/gems/2.6.0/gems/bundler-1.17.2/exe/bundle:22:in `' /usr/local/bin/bundle:23:in `load' /usr/local/bin/bundle:23:in `'
never mind, fixed it. |
Ignore hound.. sorry. |
So, do those failures make any sense to you locally? |
I changed up the rails versions a bit. |
The failures are a little baffling to me, and I'm trying to repro it locally. This might take the weekend for me to figure out. |
several tests is failing with this changed https://github.com/jejacks0n/teaspoon/actions/runs/657166943 |
As it is currently set up, if you're running Teaspoon in apps that have strong CSP settings, then all the inline JS pieces will not load causing people to be confused as to why individual tests are not running.
This patch fixes that by moving the files into their own JS files. The trick though was that we still need the suite information for the JS, so what we do here is that we add another route into the controller to custom render the JS based on the suite name (which we also use for the show view anyway).