DEV: Remove unsafe-eval from development CSP (#8569)

- Refactor source_url to avoid using eval in development
- Precompile handlebars in development
- Include template compilers when running qunit
- Remove unsafe-eval in development CSP
- Include unsafe-eval only for qunit routes in development
This commit is contained in:
David Taylor 2019-12-30 12:17:12 +00:00 committed by GitHub
parent df8444e813
commit bc4c40abd4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 33 additions and 24 deletions

View File

@ -0,0 +1 @@
//= require handlebars.runtime

View File

@ -1,8 +0,0 @@
<%
if Rails.env.development? || Rails.env.test?
require_asset ("handlebars.js")
require_asset ("ember-template-compiler.js")
else
require_asset ("handlebars.runtime.js")
end
%>

View File

@ -30,7 +30,7 @@ Discourse::Application.configure do
config.active_record.migration_error = :page_load
config.watchable_dirs['lib'] = [:rb]
config.handlebars.precompile = false
config.handlebars.precompile = true
# we recommend you use mailcatcher https://github.com/sj26/mailcatcher
config.action_mailer.smtp_settings = { address: "localhost", port: 1025 }

View File

@ -5,7 +5,7 @@ require 'discourse_iife'
Rails.application.config.assets.configure do |env|
env.register_preprocessor('application/javascript', DiscourseIIFE)
unless Rails.env.production? || ENV["DISABLE_EVAL"]
unless Rails.env.production?
require 'source_url'
env.register_postprocessor('application/javascript', SourceURL)
end

View File

@ -4,8 +4,8 @@ require 'content_security_policy/extension'
class ContentSecurityPolicy
class << self
def policy(theme_ids = [])
new.build(theme_ids)
def policy(theme_ids = [], path_info: "/")
new.build(theme_ids, path_info: path_info)
end
def base_url
@ -14,12 +14,13 @@ class ContentSecurityPolicy
attr_writer :base_url
end
def build(theme_ids)
def build(theme_ids, path_info: "/")
builder = Builder.new
Extension.theme_extensions(theme_ids).each { |extension| builder << extension }
Extension.plugin_extensions.each { |extension| builder << extension }
builder << Extension.site_setting_extension
builder << Extension.path_specific_extension(path_info)
builder.build
end

View File

@ -51,7 +51,6 @@ class ContentSecurityPolicy
"#{base_url}/mini-profiler-resources/",
*script_assets
].tap do |sources|
sources << :unsafe_eval if Rails.env.development? # TODO Remove this when we stop using `eval` in development mode
sources << 'https://www.google-analytics.com/analytics.js' if SiteSetting.ga_universal_tracking_code.present?
sources << 'https://www.googletagmanager.com/gtm.js' if SiteSetting.gtm_container_id.present?
end

View File

@ -7,6 +7,13 @@ class ContentSecurityPolicy
{ script_src: SiteSetting.content_security_policy_script_src.split('|') }
end
def path_specific_extension(path_info)
{}.tap do |obj|
for_qunit_route = !Rails.env.production? && ["/qunit", "/wizard/qunit"].include?(path_info)
obj[:script_src] = :unsafe_eval if for_qunit_route
end
end
def plugin_extensions
[].tap do |extensions|
Discourse.plugins.each do |plugin|

View File

@ -15,8 +15,9 @@ class ContentSecurityPolicy
ContentSecurityPolicy.base_url = request.host_with_port if Rails.env.development?
theme_ids = env[:resolved_theme_ids]
headers['Content-Security-Policy'] = policy(theme_ids) if SiteSetting.content_security_policy
headers['Content-Security-Policy-Report-Only'] = policy(theme_ids) if SiteSetting.content_security_policy_report_only
headers['Content-Security-Policy'] = policy(theme_ids, path_info: env["PATH_INFO"]) if SiteSetting.content_security_policy
headers['Content-Security-Policy-Report-Only'] = policy(theme_ids, path_info: env["PATH_INFO"]) if SiteSetting.content_security_policy_report_only
response
end

View File

@ -16,10 +16,9 @@ class SourceURL < Tilt::Template
end
def evaluate(scope, locals, &block)
code = +"eval("
code << data.inspect
code << " + \"\\n//# sourceURL=#{scope.logical_path}\""
code << ");\n"
code = +"\n//# BEGIN sourceURL=#{scope.logical_path}\n"
code << data
code << "\n//# END sourceURL=#{scope.logical_path}\n"
code
end
end

View File

@ -123,6 +123,12 @@ describe ContentSecurityPolicy do
Discourse.plugins.pop
end
it 'only includes unsafe-inline for qunit paths' do
expect(parse(policy(path_info: "/qunit"))['script-src']).to include("'unsafe-eval'")
expect(parse(policy(path_info: "/wizard/qunit"))['script-src']).to include("'unsafe-eval'")
expect(parse(policy(path_info: "/"))['script-src']).to_not include("'unsafe-eval'")
end
context "with a theme" do
let!(:theme) {
Fabricate(:theme).tap do |t|
@ -174,7 +180,7 @@ describe ContentSecurityPolicy do
end.to_h
end
def policy(theme_ids = [])
ContentSecurityPolicy.policy(theme_ids)
def policy(theme_ids = [], path_info: "/")
ContentSecurityPolicy.policy(theme_ids, path_info: path_info)
end
end

View File

@ -3,9 +3,7 @@
//= require env
//= require jquery.debug
//= require jquery.ui.widget
//= require handlebars
//= require ember.debug
//= require ember-template-compiler
//= require message-bus
//= require qunit/qunit/qunit
//= require ember-qunit
@ -26,6 +24,11 @@
//= require application
//= require admin
// These are not loaded in prod or development
// But we need them for testing handlebars templates in qunit
//= require handlebars
//= require ember-template-compiler
//= require sinon/pkg/sinon
//= require helpers/assertions