FIX: Add additional checks for automatic theme script CSP
- Skip absolute URLs - Skip CDN URLs - Skip invalid URLs - Properly format protocol-less URLs
This commit is contained in:
parent
6e01acb3cb
commit
f95609ae23
|
@ -62,7 +62,18 @@ class ContentSecurityPolicy
|
|||
html_fields.each(&:ensure_baked!)
|
||||
doc = html_fields.map(&:value_baked).join("\n")
|
||||
Nokogiri::HTML.fragment(doc).css('script[src]').each do |node|
|
||||
auto_script_src_extension[:script_src] << node['src']
|
||||
src = node['src']
|
||||
uri = URI(src)
|
||||
|
||||
next if GlobalSetting.cdn_url && src.starts_with?(GlobalSetting.cdn_url) # Ignore CDN urls (theme-javascripts)
|
||||
next if uri.host.nil? # Ignore same-domain scripts (theme-javascripts)
|
||||
next if uri.path.nil? # Ignore raw hosts
|
||||
|
||||
uri_string = uri.to_s.sub(/^\/\//, '') # Protocol-less CSP should not have // at beginning of URL
|
||||
|
||||
auto_script_src_extension[:script_src] << uri_string
|
||||
rescue URI::Error
|
||||
# Ignore invalid URI
|
||||
end
|
||||
|
||||
extensions << auto_script_src_extension
|
||||
|
|
|
@ -216,10 +216,20 @@ describe ContentSecurityPolicy do
|
|||
it 'is extended automatically when themes reference external scripts' do
|
||||
policy # call this first to make sure further actions clear the cache
|
||||
|
||||
theme.set_field(target: :common, name: "header", value: "<script src='https://example.com/myscript.js'/>")
|
||||
theme.set_field(target: :common, name: "header", value: <<~SCRIPT)
|
||||
<script src='https://example.com/myscript.js'/>
|
||||
<script src='//example2.com/protocol-less-script.js'/>
|
||||
<script src='domain-only.com'/>
|
||||
<script>console.log('inline script')</script>
|
||||
SCRIPT
|
||||
|
||||
theme.set_field(target: :desktop, name: "header", value: "")
|
||||
theme.save!
|
||||
|
||||
expect(parse(theme_policy)['script-src']).to include('https://example.com/myscript.js')
|
||||
expect(parse(theme_policy)['script-src']).to include('example2.com/protocol-less-script.js')
|
||||
expect(parse(theme_policy)['script-src']).not_to include('domain-only.com')
|
||||
expect(parse(theme_policy)['script-src']).not_to include(a_string_matching /^\/theme-javascripts/)
|
||||
|
||||
theme.destroy!
|
||||
|
||||
|
|
Loading…
Reference in New Issue