Protect exception reports from sensitive data.

lib/utopia/exceptions/mailer.rb

@@ -6,6 +6,8 @@
 require "net/smtp"
 require "mail"
 require "console"
+require "stringio"
+require "yaml"
 
 require_relative "../middleware"
 require_relative "../request"
@@ -24,20 +26,33 @@ class Mailer < Protocol::HTTP::Middleware
 
 			DEFAULT_FROM = (ENV["USER"] || "utopia").freeze
 			DEFAULT_SUBJECT = "%{exception} [PID %{pid} : %{cwd}]".freeze
+			ATTACHMENT_SIZE_LIMIT = 64*1024
+			SENSITIVE_FIELD = /authorization|cookie|credential|password|private[_-]?key|referer|referrer|secret|session|token|variables|api[_-]?key/i
+			REDACTED = "[REDACTED]".freeze
 
 			# @param to [String] The address to email error reports to.
 			# @param from [String] The from address for error reports.
 			# @param subject [String] The subject template which can access attributes defined by `#attributes_for`.
 			# @param delivery_method [Object] The delivery method as required by the mail gem.
-			# @param dump_environment [Boolean] Attach request attributes as `attributes.yaml` to the error report.
-			def initialize(app, to: "postmaster", from: DEFAULT_FROM, subject: DEFAULT_SUBJECT, delivery_method: LOCAL_SMTP, dump_environment: false)
+			# @param dump_body [Boolean] Attach a bounded rewindable request body to the error report.
+			# @param dump_environment [Boolean] Include application state and attach it as `state.yaml`.
+			# @param attachment_size_limit [Integer] The maximum size of each attachment.
+			# @param redact [Regexp | Nil] A pattern matching structured field names whose values should be redacted.
+			def initialize(app, to: "postmaster", from: DEFAULT_FROM, subject: DEFAULT_SUBJECT, delivery_method: LOCAL_SMTP, dump_body: false, dump_environment: false, attachment_size_limit: ATTACHMENT_SIZE_LIMIT, redact: SENSITIVE_FIELD)
 				super(app)
 
 				@to = to
 				@from = from
 				@subject = subject
 				@delivery_method = delivery_method
+				@dump_body = dump_body
 				@dump_environment = dump_environment
+				@attachment_size_limit = Integer(attachment_size_limit)
+				@redact = redact
+
+				if @attachment_size_limit < 0
+					raise ArgumentError, "attachment_size_limit must not be negative!"
+				end
 			end
 
 			# Freeze this object and its internal state.
@@ -49,7 +64,10 @@ def freeze
 				@from.freeze
 				@subject.freeze
 				@delivery_method.freeze
+				@dump_body.freeze
 				@dump_environment.freeze
+				@attachment_size_limit.freeze
+				@redact.freeze
 
 				super
 			end
@@ -100,30 +118,32 @@ def generate_backtrace(io, exception, prefix: "Exception")
 			def generate_body(exception, request)
 				io = StringIO.new
 
-				io.puts "#{request.method} #{request.url}"
-
-				# TODO embed the request body if it's textual?
-				# TODO dump and embed `utopia.variables`?
+				# Do not include the raw query string, as it may contain sensitive values:
+				io.puts "#{request.method} #{request.url.path.encoded}"
 
 				io.puts
 
 				REQUEST_ATTRIBUTES.each do |key|
-					value = request.send(key)
+					value = redact(key, request.send(key))
 					io.puts "request.#{key}: #{value.inspect}"
 				end
 
 				request.query_parameters.each do |key, value|
+					value = redact(key, value)
 					io.puts "request.query_parameters.#{key}: #{value.inspect}"
 				end
 
 				io.puts
 
 				request.headers.each do |key, value|
+					value = redact(key, value)
 					io.puts "header[#{key.inspect}]: #{value.inspect}"
 				end
 
-				self.current_state(request).each do |key, value|
-					io.puts "state.#{key}: #{value.inspect}"
+				if @dump_environment
+					filtered_state(request).each do |key, value|
+						io.puts "state.#{key}: #{value.inspect}"
+					end
 				end
 
 				io.puts
@@ -151,12 +171,14 @@ def generate_mail(exception, request)
 				mail.text_part = Mail::Part.new
 				mail.text_part.body = generate_body(exception, request)
 
-				if body = extract_body(request) and body.size > 0
-					mail.attachments["body.bin"] = body
+				if @dump_body
+					if body = extract_body(request, @attachment_size_limit)
+						mail.attachments["body.bin"] = body
+					end
 				end
 
 				if @dump_environment
-					mail.attachments["state.yaml"] = YAML.dump(self.current_state(request))
+					attach(mail, "state.yaml", YAML.dump(filtered_state(request)))
 				end
 
 				return mail
@@ -181,11 +203,51 @@ def current_state(request)
 				}
 			end
 
-			def extract_body(request)
+			def filtered_state(request)
+				redact(nil, current_state(request))
+			end
+
+			def redact(name, value)
+				if @redact && name
+					if @redact.match?(name.to_s)
+						return REDACTED
+					end
+				end
+
+				case value
+				when Hash
+					return value.to_h do |key, item|
+						[key, redact(key, item)]
+					end
+				when Array
+					return value.map{|item| redact(nil, item)}
+				else
+					return value
+				end
+			end
+
+			def attach(mail, name, content)
+				if content.bytesize <= @attachment_size_limit
+					mail.attachments[name] = content
+				end
+			end
+
+			def extract_body(request, size_limit)
 				body = request.body
 
 				if body&.rewindable? && body.rewind
-					return body.join
+					buffer = String.new.b
+
+					body.each do |chunk|
+						# Do not retain a partial body when the complete attachment would exceed the limit:
+						if chunk.bytesize > size_limit - buffer.bytesize
+							return nil
+						end
+
+						buffer << chunk
+					end
+
+					return buffer unless buffer.empty?
 				end
 			end
 		end

releases.md

@@ -4,6 +4,7 @@
 
   - **Breaking** Remove support for JavaScript packages installed in `lib/components`; use `node_modules` instead.
   - **Security** Authenticate encrypted session cookies using AES-256-GCM. Existing session cookies are invalidated.
+  - **Security** Redact sensitive exception report fields and make bounded request body attachments opt-in.
 
 ## v3.0.0
 

test/utopia/controller/base.rb

@@ -41,6 +41,7 @@
 
 	it "describes controller instances" do
 		expect(controller.to_s).to be == "#<Utopia::Controller::Base>"
+		expect(controller.inspect).to be == "#<Utopia::Controller::Base>"
 	end
 
 	it "produces semantic results for negotiated responses" do

test/utopia/exceptions/mailer.rb

@@ -36,12 +36,14 @@ def before
 		from = +"utopia@example.com"
 		template = +"%{exception}"
 		delivery_method = [:test, {}]
+		redact = /secret/
 		middleware = subject.new(
 			Protocol::HTTP::Middleware::NotFound,
 			to: to,
 			from: from,
 			subject: template,
 			delivery_method: delivery_method,
+			redact: redact,
 		)
 
 		expect(middleware.freeze).to be_equal(middleware)
@@ -52,6 +54,7 @@ def before
 		expect(from).to be(:frozen?)
 		expect(template).to be(:frozen?)
 		expect(delivery_method).to be(:frozen?)
+		expect(redact).to be(:frozen?)
 	end
 
 	it "should send an email to report the failure" do
@@ -86,18 +89,116 @@ def before
 		expect(output.string).to be(:include?, "Caused by Object: Inner failure")
 	end
 
-	it "attaches buffered request bodies and environment state" do
+	it "attaches bounded request bodies and filtered environment state" do
 		request = Utopia::Request["POST", "/submit", {}, ["Hello World!"]]
+		request.session = {token: "session-secret"}
+		request.variables = {password: "variable-secret"}
 		mailer = subject.new(
 			Protocol::HTTP::Middleware::NotFound,
 			delivery_method: nil,
+			dump_body: true,
 			dump_environment: true,
 		)
 
 		mail = mailer.send(:generate_mail, RuntimeError.new("Failure"), request)
 
 		expect(mail.attachments["body.bin"].decoded).to be == "Hello World!"
-		expect(mail.attachments["state.yaml"]).not.to be_nil
+		expect(mail.attachments["state.yaml"].decoded).to be(:include?, "[REDACTED]")
+		expect(mail.attachments["state.yaml"].decoded).not.to be(:include?, "session-secret")
+		expect(mail.attachments["state.yaml"].decoded).not.to be(:include?, "variable-secret")
+	end
+
+	it "does not attach request bodies by default" do
+		request = Utopia::Request["POST", "/submit", {}, ["Hello World!"]]
+		mailer = subject.new(Protocol::HTTP::Middleware::NotFound, delivery_method: nil)
+
+		mail = mailer.send(:generate_mail, RuntimeError.new("Failure"), request)
+
+		expect(mail.attachments["body.bin"]).to be_nil
+	end
+
+	it "does not attach environment state above the limit" do
+		request = Utopia::Request["GET", "/"]
+		mailer = subject.new(
+			Protocol::HTTP::Middleware::NotFound,
+			delivery_method: nil,
+			dump_environment: true,
+			attachment_size_limit: 0,
+		)
+
+		mail = mailer.send(:generate_mail, RuntimeError.new("Failure"), request)
+
+		expect(mail.attachments["state.yaml"]).to be_nil
+	end
+
+	it "rejects a negative attachment size limit" do
+		expect do
+			subject.new(
+				Protocol::HTTP::Middleware::NotFound,
+				attachment_size_limit: -1,
+			)
+		end.to raise_exception(ArgumentError, message: be =~ /must not be negative/)
+	end
+
+	it "redacts sensitive request fields" do
+		request = Utopia::Request[
+			"GET",
+			"/submit?token=query-secret&name=Samuel",
+			{
+				"authorization" => "Bearer header-secret",
+				"referer" => "https://example.com/?token=referrer-secret",
+				"x-request-id" => "public-request-id",
+			},
+		]
+		mailer = subject.new(Protocol::HTTP::Middleware::NotFound, delivery_method: nil)
+
+		mail = mailer.send(:generate_mail, RuntimeError.new("Failure"), request)
+		body = mail.text_part.decoded
+
+		expect(body).to be(:include?, "GET /submit")
+		expect(body).to be(:include?, "public-request-id")
+		expect(body).to be(:include?, "Samuel")
+		expect(body).to be(:include?, "[REDACTED]")
+		expect(body).not.to be(:include?, "query-secret")
+		expect(body).not.to be(:include?, "header-secret")
+		expect(body).not.to be(:include?, "referrer-secret")
+		expect(body).not.to be(:include?, "state.session")
+	end
+
+	it "redacts sensitive fields nested in arrays" do
+		mailer = subject.new(Protocol::HTTP::Middleware::NotFound, delivery_method: nil)
+		value = [{"token" => "secret"}, "public"]
+
+		expect(mailer.send(:redact, nil, value)).to be == [
+			{"token" => "[REDACTED]"},
+			"public",
+		]
+	end
+
+	with "a body attachment size limit" do
+		def generate_mail(body, attachment_size_limit:)
+			request = Utopia::Request["POST", "/submit", {}, [body]]
+			mailer = subject.new(
+				Protocol::HTTP::Middleware::NotFound,
+				delivery_method: nil,
+				dump_body: true,
+				attachment_size_limit: attachment_size_limit,
+			)
+
+			return mailer.send(:generate_mail, RuntimeError.new("Failure"), request)
+		end
+
+		it "attaches a body at the limit" do
+			mail = generate_mail("1234", attachment_size_limit: 4)
+
+			expect(mail.attachments["body.bin"].decoded).to be == "1234"
+		end
+
+		it "does not attach a body above the limit" do
+			mail = generate_mail("12345", attachment_size_limit: 4)
+
+			expect(mail.attachments["body.bin"]).to be_nil
+		end
 	end
 
 	it "does not propagate delivery failures" do
@@ -139,7 +240,7 @@ def deliver!(mail)
 		request.body.read
 		mailer = subject.new(Protocol::HTTP::Middleware::NotFound, delivery_method: nil)
 
-		expect(mailer.send(:extract_body, request)).to be == "Hello World!"
+		expect(mailer.send(:extract_body, request, 12)).to be == "Hello World!"
 	end
 
 	it "does not extract streaming request bodies" do
@@ -148,6 +249,6 @@ def body.rewindable? = false
 		request = Struct.new(:body).new(body)
 		mailer = subject.new(Protocol::HTTP::Middleware::NotFound, delivery_method: nil)
 
-		expect(mailer.send(:extract_body, request)).to be_nil
+		expect(mailer.send(:extract_body, request, 1024)).to be_nil
 	end
 end