Skip to content
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

Remove skipping CSP nonce on unsafe-inline #1010

Merged
merged 1 commit into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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