Skip to content

Commit

Permalink
fix: temporarily redact webhook response body from UI for security pu…
Browse files Browse the repository at this point in the history
…rposes
  • Loading branch information
bethesque committed May 29, 2018
1 parent cf3bc3a commit becf20c
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,15 @@ def body
represented.body
end
end

end

property :error, :extend => ErrorDecorator
property :response, :extend => HTTPResponseDecorator
property :message, exec_context: :decorator
# property :error, :extend => ErrorDecorator
# property :response, :extend => HTTPResponseDecorator

def message
"Webhook response has been redacted temporarily for security purposes. Please see the Pact Broker application logs for the response body."
end

link :webhook do | options |
{
Expand Down
23 changes: 12 additions & 11 deletions lib/pact_broker/domain/webhook_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,19 @@ def log_response response, execution_logger
execution_logger.info(" ")
logger.info "Received response for webhook #{uuid} status=#{response.code}"
execution_logger.info "HTTP/#{response.http_version} #{response.code} #{response.message}"
response.each_header do | header |
execution_logger.info "#{header.split("-").collect(&:capitalize).join('-')}: #{response[header]}"
end
#response.each_header do | header |
# execution_logger.info "#{header.split("-").collect(&:capitalize).join('-')}: #{response[header]}"
#end
logger.debug "body=#{response.body}"
safe_body = nil
if response.body
safe_body = response.body.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')
if response.body != safe_body
execution_logger.debug "Note that invalid UTF-8 byte sequences were removed from response body before saving the logs"
end
end
execution_logger.info safe_body
# safe_body = nil
# if response.body
# safe_body = response.body.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')
# if response.body != safe_body
# execution_logger.debug "Note that invalid UTF-8 byte sequences were removed from response body before saving the logs"
# end
# end
#execution_logger.info safe_body
execution_logger.info "Webhook response has been redacted temporarily for security purposes. Please see the Pact Broker application logs for the response body."
end

def log_completion_message options, execution_logger, success
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ module Decorators
expect(subject[:_links][:webhook][:href]).to eq 'http://example.org/webhooks/some-uuid'
end

context "when there is an error" do
it "includes a message about the response being redacted" do
expect(subject[:message]).to match /redacted/
end

context "when there is an error", pending: "temporarily disabled" do
let(:error) { double('error', message: 'message', backtrace: ['blah','blah']) }

it "includes the message" do
Expand All @@ -42,7 +46,7 @@ module Decorators
end
end

context "when there is a response" do
context "when there is a response", pending: "temporarily disabled" do
it "includes the response code" do
expect(subject[:response][:status]).to eq 200
end
Expand Down
21 changes: 16 additions & 5 deletions spec/lib/pact_broker/domain/webhook_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ module Domain
before do
allow(PactBroker::Api::PactBrokerUrls).to receive(:pact_url).and_return('http://example.org/pact-url')
allow(PactBroker.configuration).to receive(:base_url).and_return('http://example.org')
allow(PactBroker.logger).to receive(:info).and_call_original
allow(PactBroker.logger).to receive(:debug).and_call_original
allow(PactBroker.logger).to receive(:warn).and_call_original
end

let(:username) { nil }
Expand Down Expand Up @@ -121,6 +124,14 @@ module Domain
subject.execute(pact, options)
end

it "does not write the response body to the exeuction log for security purposes" do
expect(logs).to_not include "An error"
end

it "logs a message about why there is no response information" do
expect(logs).to include "Webhook response has been redacted temporarily for security purposes"
end

describe "execution logs" do

it "logs the request method and path" do
Expand All @@ -143,12 +154,12 @@ module Domain
expect(logs).to include "HTTP/1.0 200"
end

it "logs the response headers" do
expect(logs).to include "Content-Type: text/foo, blah"
it "does not log the response headers" do
expect(logs).to_not include "Content-Type: text/foo, blah"
end

it "logs the response body" do
expect(logs).to include "respbod"
it "does not log the response body" do
expect(logs).to_not include "respbod"
end

context "when the response code is a success" do
Expand Down Expand Up @@ -280,7 +291,7 @@ module Domain
end
end

context "when the response body contains a non UTF-8 character" do
context "when the response body contains a non UTF-8 character", pending: "execution logs disabled temporarily for security purposes" do
let!(:http_request) do
stub_request(:post, "http://example.org/hook").
to_return(:status => 200, :body => "This has some \xC2 invalid chars")
Expand Down

0 comments on commit becf20c

Please sign in to comment.