Skip to content

Commit 4f58080

Browse files
Merge pull request #497 from BetterErrors/feature/content-security-policy
Add Content Security Policy
2 parents 133f795 + b9d9ab7 commit 4f58080

File tree

7 files changed

+141
-45
lines changed

7 files changed

+141
-45
lines changed

lib/better_errors/editor.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ def url(raw_path, line)
8484
url_proc.call(file, line)
8585
end
8686

87+
def scheme
88+
url('/fake', 42).sub(/:.*/, ':')
89+
end
90+
8791
private
8892

8993
attr_reader :url_proc

lib/better_errors/error_page.rb

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
module BetterErrors
66
# @private
77
class ErrorPage
8+
VariableInfo = Struct.new(:frame, :editor_url, :rails_params, :rack_session, :start_time)
9+
810
def self.template_path(template_name)
911
File.expand_path("../templates/#{template_name}.erb", __FILE__)
1012
end
@@ -13,6 +15,15 @@ def self.template(template_name)
1315
Erubi::Engine.new(File.read(template_path(template_name)), escape: true)
1416
end
1517

18+
def self.render_template(template_name, locals)
19+
locals.send(:eval, self.template(template_name).src)
20+
rescue => e
21+
# Fix the backtrace, which doesn't identify the template that failed (within Better Errors).
22+
# We don't know the line number, so just injecting the template path has to be enough.
23+
e.backtrace.unshift "#{self.template_path(template_name)}:0"
24+
raise
25+
end
26+
1627
attr_reader :exception, :env, :repls
1728

1829
def initialize(exception, env)
@@ -26,20 +37,21 @@ def id
2637
@id ||= SecureRandom.hex(8)
2738
end
2839

29-
def render(template_name = "main", csrf_token = nil)
30-
binding.eval(self.class.template(template_name).src)
31-
rescue => e
32-
# Fix the backtrace, which doesn't identify the template that failed (within Better Errors).
33-
# We don't know the line number, so just injecting the template path has to be enough.
34-
e.backtrace.unshift "#{self.class.template_path(template_name)}:0"
35-
raise
40+
def render_main(csrf_token, csp_nonce)
41+
frame = backtrace_frames[0]
42+
first_frame_variable_info = VariableInfo.new(frame, editor_url(frame), rails_params, rack_session, Time.now.to_f)
43+
self.class.render_template('main', binding)
44+
end
45+
46+
def render_text
47+
self.class.render_template('text', binding)
3648
end
3749

3850
def do_variables(opts)
3951
index = opts["index"].to_i
40-
@frame = backtrace_frames[index]
41-
@var_start_time = Time.now.to_f
42-
{ html: render("variable_info") }
52+
frame = backtrace_frames[index]
53+
variable_info = VariableInfo.new(frame, editor_url(frame), rails_params, rack_session, Time.now.to_f)
54+
{ html: self.class.render_template("variable_info", variable_info) }
4355
end
4456

4557
def do_eval(opts)
@@ -113,19 +125,19 @@ def request_path
113125
env["PATH_INFO"]
114126
end
115127

116-
def html_formatted_code_block(frame)
128+
def self.html_formatted_code_block(frame)
117129
CodeFormatter::HTML.new(frame.filename, frame.line).output
118130
end
119131

120-
def text_formatted_code_block(frame)
132+
def self.text_formatted_code_block(frame)
121133
CodeFormatter::Text.new(frame.filename, frame.line).output
122134
end
123135

124136
def text_heading(char, str)
125137
str + "\n" + char*str.size
126138
end
127139

128-
def inspect_value(obj)
140+
def self.inspect_value(obj)
129141
if BetterErrors.ignored_classes.include? obj.class.name
130142
"<span class='unsupported'>(Instance of ignored class. "\
131143
"#{obj.class.name ? "Remove #{CGI.escapeHTML(obj.class.name)} from" : "Modify"}"\

lib/better_errors/middleware.rb

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,13 @@ def protected_app_call(env)
9494
def show_error_page(env, exception=nil)
9595
request = Rack::Request.new(env)
9696
csrf_token = request.cookies[CSRF_TOKEN_COOKIE_NAME] || SecureRandom.uuid
97+
csp_nonce = SecureRandom.base64(12)
9798

9899
type, content = if @error_page
99100
if text?(env)
100-
[ 'plain', @error_page.render('text') ]
101+
[ 'plain', @error_page.render_text ]
101102
else
102-
[ 'html', @error_page.render('main', csrf_token) ]
103+
[ 'html', @error_page.render_main(csrf_token, csp_nonce) ]
103104
end
104105
else
105106
[ 'html', no_errors_page ]
@@ -110,7 +111,22 @@ def show_error_page(env, exception=nil)
110111
status_code = ActionDispatch::ExceptionWrapper.new(env, exception).status_code
111112
end
112113

113-
response = Rack::Response.new(content, status_code, { "Content-Type" => "text/#{type}; charset=utf-8" })
114+
headers = {
115+
"Content-Type" => "text/#{type}; charset=utf-8",
116+
"Content-Security-Policy" => [
117+
"default-src 'none'",
118+
# Specifying nonce makes a modern browser ignore 'unsafe-inline' which could still be set
119+
# for older browsers without nonce support.
120+
# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src
121+
"script-src 'self' 'nonce-#{csp_nonce}' 'unsafe-inline'",
122+
# Inline style is required by the syntax highlighter.
123+
"style-src 'self' 'unsafe-inline'",
124+
"connect-src 'self'",
125+
"navigate-to 'self' #{BetterErrors.editor.scheme}",
126+
].join('; '),
127+
}
128+
129+
response = Rack::Response.new(content, status_code, headers)
114130

115131
unless request.cookies[CSRF_TOKEN_COOKIE_NAME]
116132
response.set_cookie(CSRF_TOKEN_COOKIE_NAME, value: csrf_token, path: "/", httponly: true, same_site: :strict)

lib/better_errors/templates/main.erb

Lines changed: 74 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<head>
44
<title><%= exception_type %> at <%= request_path %></title>
55
</head>
6-
<body>
6+
<body class="better-errors-javascript-not-loaded">
77
<%# Stylesheets are placed in the <body> for Turbolinks compatibility. %>
88
<style>
99
/* Basic reset */
@@ -107,13 +107,18 @@
107107
}
108108

109109
.frame_info {
110+
display: none;
111+
110112
right: 0;
111113
left: 40%;
112114

113115
padding: 20px;
114116
padding-left: 10px;
115117
margin-left: 30px;
116118
}
119+
.frame_info.current {
120+
display: block;
121+
}
117122
}
118123

119124
nav.sidebar {
@@ -227,6 +232,10 @@
227232
* Navigation
228233
* --------------------------------------------------------------------- */
229234

235+
.better-errors-javascript-not-loaded .backtrace .tabs {
236+
display: none;
237+
}
238+
230239
nav.tabs {
231240
border-bottom: solid 1px #ddd;
232241

@@ -411,6 +420,18 @@
411420
* Display area
412421
* --------------------------------------------------------------------- */
413422

423+
p.no-javascript-notice {
424+
margin-bottom: 1em;
425+
padding: 1em;
426+
border: 2px solid #e00;
427+
}
428+
.better-errors-javascript-loaded .no-javascript-notice {
429+
display: none;
430+
}
431+
.no-inline-style-notice {
432+
display: none;
433+
}
434+
414435
.trace_info {
415436
background: #fff;
416437
padding: 6px;
@@ -468,6 +489,10 @@
468489
font-weight: 200;
469490
}
470491

492+
.better-errors-javascript-not-loaded .be-repl {
493+
display: none;
494+
}
495+
471496
.code, .be-console, .unavailable {
472497
background: #fff;
473498
padding: 5px;
@@ -598,6 +623,9 @@
598623
.console-has-been-used .live-console-hint {
599624
display: none;
600625
}
626+
.better-errors-javascript-not-loaded .live-console-hint {
627+
display: none;
628+
}
601629

602630
.hint:before {
603631
content: '\25b2';
@@ -701,7 +729,7 @@
701729
</style>
702730

703731
<%# IE8 compatibility crap %>
704-
<script>
732+
<script nonce="<%= csp_nonce %>">
705733
(function() {
706734
var elements = ["section", "nav", "header", "footer", "audio"];
707735
for (var i = 0; i < elements.length; i++) {
@@ -715,7 +743,7 @@
715743
rendered in the host app's layout. Let's empty out the styles of the
716744
host app.
717745
%>
718-
<script>
746+
<script nonce="<%= csp_nonce %>">
719747
if (window.Turbolinks) {
720748
for(var i=0; i < document.styleSheets.length; i++) {
721749
if(document.styleSheets[i].href)
@@ -740,6 +768,15 @@
740768
}
741769
</script>
742770

771+
<p class='no-inline-style-notice'>
772+
<strong>
773+
Better Errors can't apply inline style<span class='no-javascript-notice'> (or run Javascript)</span>,
774+
possibly because you have a Content Security Policy along with Turbolinks.
775+
But you can
776+
<a href='/__better_errors' target="_blank">open the interactive console in a new tab/window</a>.
777+
</strong>
778+
</p>
779+
743780
<div class='top'>
744781
<header class="exception">
745782
<h2><strong><%= exception_type %></strong> <span>at <%= request_path %></span></h2>
@@ -786,21 +823,37 @@
786823
</ul>
787824
</nav>
788825

789-
<% backtrace_frames.each_with_index do |frame, index| %>
790-
<div class="frame_info" id="frame_info_<%= index %>" style="display:none;"></div>
791-
<% end %>
826+
<div class="frameInfos">
827+
<div class="frame_info current" data-frame-idx="0">
828+
<p class='no-javascript-notice'>
829+
Better Errors can't run Javascript here<span class='no-inline-style-notice'> (or apply inline style)</span>,
830+
possibly because you have a Content Security Policy along with Turbolinks.
831+
But you can
832+
<a href='/__better_errors' target="_blank">open the interactive console in a new tab/window</a>.
833+
</p>
834+
<!-- this is enough information to show something in case JS doesn't get to load -->
835+
<%== ErrorPage.render_template('variable_info', first_frame_variable_info) %>
836+
</div>
837+
</div>
792838
</section>
793839
</body>
794-
<script>
840+
<script nonce="<%= csp_nonce %>">
795841
(function() {
796842

797843
var OID = "<%= id %>";
798844
var csrfToken = "<%= csrf_token %>";
799845

800846
var previousFrame = null;
801-
var previousFrameInfo = null;
802847
var allFrames = document.querySelectorAll("ul.frames li");
803-
var allFrameInfos = document.querySelectorAll(".frame_info");
848+
var frameInfos = document.querySelector(".frameInfos");
849+
850+
document.querySelector('body').classList.remove("better-errors-javascript-not-loaded");
851+
document.querySelector('body').classList.add("better-errors-javascript-loaded");
852+
853+
var noJSNotices = document.querySelectorAll('.no-javascript-notice');
854+
for(var i = 0; i < noJSNotices.length; i++) {
855+
noJSNotices[i].remove();
856+
}
804857

805858
function apiCall(method, opts, cb) {
806859
var req = new XMLHttpRequest();
@@ -974,17 +1027,25 @@
9741027
};
9751028

9761029
function switchTo(el) {
977-
if(previousFrameInfo) previousFrameInfo.style.display = "none";
978-
previousFrameInfo = el;
1030+
var currentFrameInfo = document.querySelectorAll('.frame_info.current');
1031+
for(var i = 0; i < currentFrameInfo.length; i++) {
1032+
currentFrameInfo[i].className = "frame_info";
1033+
}
9791034

980-
el.style.display = "block";
1035+
el.className = "frame_info current";
9811036

9821037
var replInput = el.querySelector('.be-console input');
9831038
if (replInput) replInput.focus();
9841039
}
9851040

9861041
function selectFrameInfo(index) {
987-
var el = allFrameInfos[index];
1042+
var el = document.querySelector(".frame_info[data-frame-idx='" + index + "']")
1043+
if (!el) {
1044+
el = document.createElement("div");
1045+
el.className = "frame_info";
1046+
el.setAttribute('data-frame-idx', index);
1047+
frameInfos.appendChild(el);
1048+
}
9881049
if(el) {
9891050
if (el.loaded) {
9901051
return switchTo(el);

lib/better_errors/templates/text.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<%== text_heading("-", "%s, line %i" % [first_frame.pretty_path, first_frame.line]) %>
1010

1111
``` ruby
12-
<%== text_formatted_code_block(first_frame) %>```
12+
<%== ErrorPage.text_formatted_code_block(first_frame) %>```
1313

1414
App backtrace
1515
-------------

0 commit comments

Comments
 (0)