Skip to content

Commit

Permalink
Remove skipping CSP nonce on unsafe-inline (#1010)
Browse files Browse the repository at this point in the history
The current implementation for Rails's CSP nonce functionality
intentionally skips adding a nonce to the script tag if the script-src
directive includes `unsafe-inline`. However, using both a nonce and
unsafe-inline at the same time is perfectly valid (and indeed sensible)
behaviour. It allows the app to maintain some level of backwards
compatibility with browsers that support CSP1 but not CSP2, c.f.:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src

- Remove special handling of nonces when `unsafe-inline` is specified
  in the `script-src` directive
  • Loading branch information
csutter authored Dec 11, 2020
1 parent 310a9d1 commit 71c2736
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 44 deletions.
7 changes: 1 addition & 6 deletions lib/rollbar/middleware/js.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ def rails5_nonce(env)
req.respond_to?(:content_security_policy) &&
req.content_security_policy &&
req.content_security_policy.directives['script-src'] &&
!req.content_security_policy.directives['script-src'].include?("'unsafe-inline'") &&
req.content_security_policy_nonce
end

Expand Down Expand Up @@ -224,16 +223,12 @@ def find_csp
end

def csp_needs_nonce?(csp)
!opt_out?(csp) && !unsafe_inline?(csp)
!opt_out?(csp)
end

def opt_out?(_csp)
raise NotImplementedError
end

def unsafe_inline?(csp)
csp[:script_src].to_a.include?("'unsafe-inline'")
end
end

class SecureHeadersFalse < SecureHeadersResolver
Expand Down
14 changes: 0 additions & 14 deletions spec/rollbar/middleware/js_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,6 @@
expect(new_body).to include(snippet)
end

it 'renders the snippet in the response without nonce if SecureHeaders script_src includes \'unsafe-inline\'' do
SecureHeadersMocks::CSP.config = {
:opt_out? => false,
:script_src => %w['unsafe-inline'] # rubocop:disable Lint/PercentStringArray
}

_, _, response = subject.call(env)
new_body = response.body.join

expect(new_body).to include('<script type="text/javascript">')
expect(new_body).to include("var _rollbarConfig = #{json_options};")
expect(new_body).to include(snippet)
end

it 'renders the snippet in the response without nonce if SecureHeaders CSP is OptOut' do
SecureHeadersMocks::CSP.config = {
:opt_out? => true
Expand Down
25 changes: 1 addition & 24 deletions spec/rollbar/plugins/rails_js_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ def configure_csp(mode)
nonce_present
elsif mode == :script_src_not_present
script_src_not_present
elsif mode == :unsafe_inline
unsafe_inline
else
raise 'Unknown CSP mode'
end
Expand All @@ -53,14 +51,6 @@ def script_src_not_present
end
end

def unsafe_inline
# Browser behavior is undefined when unsafe_inline and the nonce are both present.
# The app should never set both, but if they do, our best behavior is to not use the nonce.
Rails.application.config.content_security_policy do |policy|
policy.script_src :self, :unsafe_inline
end
end

def nonce(response)
response.request.content_security_policy_nonce
end
Expand Down Expand Up @@ -107,20 +97,7 @@ def reset_rails_config
include_examples 'adds the snippet'
end

context 'when unsafe_inline is present' do
let(:nonce_mode) { :unsafe_inline }

it 'renders the snippet and config in the response with nonce in script tag' do
get '/test_rollbar_js'

expect(response.body).to_not include %[<script type="text/javascript" nonce="#{nonce(response)}">]
expect(response.body).to include '<script type="text/javascript">'
end

include_examples 'adds the snippet'
end

context 'when scp nonce is present' do
context 'when CSP nonce is present' do
let(:nonce_mode) { :nonce_present }

it 'renders the snippet and config in the response with nonce in script tag' do
Expand Down

0 comments on commit 71c2736

Please sign in to comment.