fix: temporarily redact webhook response body from UI for security purposes

Author: bethesque

lib/pact_broker/api/decorators/webhook_execution_result_decorator.rb

@@ -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 |
           {

lib/pact_broker/domain/webhook_request.rb

@@ -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

spec/lib/pact_broker/api/decorators/webhook_execution_result_decorator_spec.rb

@@ -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
@@ -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

spec/lib/pact_broker/domain/webhook_request_spec.rb

@@ -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 }
@@ -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
@@ -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
@@ -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")