feat(webhook status): redact authorization headers in webhook logs

Author: bethesque

lib/pact_broker/domain/webhook_request.rb

@@ -3,6 +3,7 @@
 require 'pact_broker/logging'
 require 'pact_broker/messages'
 require 'net/http'
+require 'pact_broker/webhooks/redact_logs'
 
 module PactBroker
 
@@ -56,7 +57,7 @@ def execute
           execution_logger.info "HTTP/1.1 #{method.upcase} #{url_with_credentials}"
 
           headers.each_pair do | name, value |
-            execution_logger.info "#{name}: #{value}"
+            execution_logger.info Webhooks::RedactLogs.call("#{name}: #{value}")
             req[name] = value
           end
 
@@ -91,9 +92,8 @@ def execute
 
         rescue StandardError => e
           logger.error "Error executing webhook #{uuid} #{e.class.name} - #{e.message}"
-          execution_logger.error "Error executing webhook #{uuid} #{e.class.name} - #{e.message}"
           logger.error e.backtrace.join("\n")
-          execution_logger.error e.backtrace.join("\n")
+          execution_logger.error "Error executing webhook #{uuid} #{e.class.name} - #{e.message}"
           WebhookExecutionResult.new(nil, logs.string, e)
         end
       end
@@ -118,7 +118,5 @@ def url_with_credentials
         u
       end
     end
-
   end
-
 end

lib/pact_broker/webhooks/redact_logs.rb

@@ -0,0 +1,10 @@
+module PactBroker
+  module Webhooks
+    class RedactLogs
+      def self.call logs
+        logs.gsub(/(Authorization: )(.*)/i,'\1[REDACTED]')
+            .gsub(/(Token: )(.*)/i,'\1[REDACTED]')
+      end
+    end
+  end
+end

spec/lib/pact_broker/domain/webhook_request_spec.rb

@@ -19,7 +19,7 @@ module Domain
         WebhookRequest.new(
           method: 'post',
           url: url,
-          headers: {'Content-Type' => 'text/plain'},
+          headers: {'Content-Type' => 'text/plain', 'Authorization' => 'foo'},
           username: username,
           password: password,
           body: body)
@@ -73,9 +73,6 @@ module Domain
         end
 
         describe "execution logs" do
-          before do
-
-          end
 
           let(:logs) { subject.execute.logs }
 
@@ -87,6 +84,10 @@ module Domain
             expect(logs).to include "Content-Type: text/plain"
           end
 
+          it "redacts potentially sensitive headers" do
+            expect(logs).to include "Authorization: [REDACTED]"
+          end
+
           it "logs the request body" do
             expect(logs).to include body
           end

spec/lib/pact_broker/webhooks/redact_logs_spec.rb

@@ -0,0 +1,49 @@
+require 'pact_broker/webhooks/redact_logs'
+
+module PactBroker
+  module Webhooks
+    describe RedactLogs do
+      describe ".call" do
+        let(:string) do
+          "Authorization: foo\nX-Thing: bar"
+        end
+
+        let(:x_auth_string) do
+          "X-Authorization: bar foo\nX-Thing: bar"
+        end
+
+        let(:x_auth_token) do
+          "X-Auth-Token: bar foo\nX-Thing: bar"
+        end
+
+        let(:x_authorization_token) do
+          "X-Authorization-Token: bar foo\nX-Thing: bar"
+        end
+
+        let(:string_lower) do
+          "authorization: foo\nX-Thing: bar"
+        end
+
+        it "hides the value of the Authorization header" do
+          expect(RedactLogs.call(string)).to eq "Authorization: [REDACTED]\nX-Thing: bar"
+        end
+
+        it "hides the value of the X-Authorization header" do
+          expect(RedactLogs.call(x_auth_string)).to eq "X-Authorization: [REDACTED]\nX-Thing: bar"
+        end
+
+        it "hides the value of the X-Auth-Token header" do
+          expect(RedactLogs.call(x_auth_token)).to eq "X-Auth-Token: [REDACTED]\nX-Thing: bar"
+        end
+
+        it "hides the value of the X-Authorization-Token header" do
+          expect(RedactLogs.call(x_authorization_token)).to eq "X-Authorization-Token: [REDACTED]\nX-Thing: bar"
+        end
+
+        it "hides the value of the authorization header" do
+          expect(RedactLogs.call(string_lower)).to eq "authorization: [REDACTED]\nX-Thing: bar"
+        end
+      end
+    end
+  end
+end

spec/spec_helper.rb

@@ -5,22 +5,22 @@
 RACK_ENV = 'test'
 
 $: << File.expand_path("../../", __FILE__)
-require 'rack/test'
+
 require 'db'
-require 'pact_broker/api'
 require 'tasks/database'
+require 'pact_broker/db'
+raise "Wrong environment!!! Don't run this script!! ENV['RACK_ENV'] is #{ENV['RACK_ENV']} and RACK_ENV is #{RACK_ENV}" if ENV['RACK_ENV'] != 'test' || RACK_ENV != 'test'
+PactBroker::DB.connection = PactBroker::Database.database = DB::PACT_BROKER_DB
+
+require 'rack/test'
+require 'pact_broker/api'
 require 'rspec/its'
 
 Dir.glob("./spec/support/**/*.rb") { |file| require file  }
 
 I18n.config.enforce_available_locales = false
 
 RSpec.configure do | config |
-  config.before :suite do
-    raise "Wrong environment!!! Don't run this script!! ENV['RACK_ENV'] is #{ENV['RACK_ENV']} and RACK_ENV is #{RACK_ENV}" if ENV['RACK_ENV'] != 'test' || RACK_ENV != 'test'
-    PactBroker::DB.connection = PactBroker::Database.database = DB::PACT_BROKER_DB
-  end
-
   config.before :each do
     PactBroker.reset_configuration
   end