feat: validate webhook host against configurable list on creation

Author: bethesque

lib/pact_broker/api/contracts/webhook_contract.rb

@@ -1,5 +1,6 @@
 require 'reform'
 require 'reform/form'
+require 'pact_broker/webhooks/check_host_whitelist'
 
 module PactBroker
   module Api
@@ -41,7 +42,8 @@ def self.messages
                   en: {
                     errors: {
                       allowed_webhook_method?: http_method_error_message,
-                      allowed_webhook_scheme?: scheme_error_message
+                      allowed_webhook_scheme?: scheme_error_message,
+                      allowed_webhook_host?: host_error_message
                     }
                   }
                 )
@@ -56,7 +58,11 @@ def self.http_method_error_message
               end
 
               def self.scheme_error_message
-                "must be #{PactBroker.configuration.webhook_scheme_whitelist.join(", ")}. See /doc/webhooks#whitelist for more information."
+                "scheme must be #{PactBroker.configuration.webhook_scheme_whitelist.join(", ")}. See /doc/webhooks#whitelist for more information."
+              end
+
+              def self.host_error_message
+                "host must be in the whitelist #{PactBroker.configuration.webhook_host_whitelist.join(",")}. See /doc/webhooks#whitelist for more information."
               end
 
               def valid_method?(http_method)
@@ -82,10 +88,22 @@ def allowed_webhook_scheme?(url)
                   scheme.downcase == allowed_scheme.downcase
                 end
               end
+
+              def allowed_webhook_host?(url)
+                if host_whitelist.any?
+                  PactBroker::Webhooks::CheckHostWhitelist.call(URI(url).host, host_whitelist).any?
+                else
+                  true
+                end
+              end
+
+              def host_whitelist
+                PactBroker.configuration.webhook_host_whitelist
+              end
             end
 
             required(:http_method).filled(:valid_method?, :allowed_webhook_method?)
-            required(:url).filled(:valid_url?, :allowed_webhook_scheme?)
+            required(:url).filled(:valid_url?, :allowed_webhook_scheme?, :allowed_webhook_host?)
           end
         end
 

lib/pact_broker/configuration.rb

@@ -30,10 +30,10 @@ class Configuration
     attr_accessor :validate_database_connection_config, :enable_diagnostic_endpoints, :version_parser, :sha_generator
     attr_accessor :use_case_sensitive_resource_names, :order_versions_by_date
     attr_accessor :check_for_potential_duplicate_pacticipant_names
-    attr_accessor :webhook_http_method_whitelist, :webhook_scheme_whitelist
+    attr_accessor :webhook_http_method_whitelist, :webhook_scheme_whitelist, :webhook_host_whitelist
+    attr_accessor :webhook_retry_schedule
     attr_accessor :semver_formats
     attr_accessor :enable_public_badge_access, :shields_io_base_url
-    attr_accessor :webhook_retry_schedule
     attr_accessor :disable_ssl_verification
     attr_accessor :base_equality_only_on_content_that_affects_verification_results
     attr_reader :api_error_reporters
@@ -77,6 +77,7 @@ def self.default_configuration
       config.disable_ssl_verification = false
       config.webhook_http_method_whitelist = ['POST']
       config.webhook_scheme_whitelist = ['https']
+      config.webhook_host_whitelist = []
       config
     end
 

lib/pact_broker/webhooks/check_host_whitelist.rb

@@ -0,0 +1,22 @@
+module PactBroker
+  module Webhooks
+    class CheckHostWhitelist
+
+      def self.call(host, whitelist = PactBroker.configuration.webhook_host_whitelist)
+        whitelist.select{ | whitelist_host | match?(host, whitelist_host) }
+      end
+
+      def self.match?(host, whitelist_host)
+        if whitelist_host.is_a?(Regexp)
+          host =~ whitelist_host
+        else
+          begin
+            IPAddr.new(whitelist_host) === IPAddr.new(host)
+          rescue IPAddr::Error
+            host == whitelist_host
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb

@@ -21,10 +21,14 @@ def valid_webhook_with
         describe "errors" do
           before do
             PactBroker.configuration.webhook_http_method_whitelist = webhook_http_method_whitelist
+            PactBroker.configuration.webhook_host_whitelist = webhook_host_whitelist
+            allow(PactBroker::Webhooks::CheckHostWhitelist).to receive(:call).and_return(whitelist_matches)
             subject.validate(hash)
           end
 
           let(:webhook_http_method_whitelist) { ['POST'] }
+          let(:whitelist_matches) { ['foo'] }
+          let(:webhook_host_whitelist) { [] }
 
           context "with valid fields" do
             it "is empty" do
@@ -92,7 +96,7 @@ def valid_webhook_with
             end
 
             it "contains an error for the URL" do
-              expect(subject.errors[:"request.url"]).to include "must be https. See /doc/webhooks#whitelist for more information."
+              expect(subject.errors[:"request.url"]).to include "scheme must be https. See /doc/webhooks#whitelist for more information."
             end
           end
 
@@ -118,6 +122,28 @@ def valid_webhook_with
             end
           end
 
+          context "when the host whitelist is empty" do
+            it "does not attempt to validate the host" do
+              expect(PactBroker::Webhooks::CheckHostWhitelist).to_not have_received(:call)
+            end
+          end
+
+          context "when the host whitelist is populated" do
+            let(:webhook_host_whitelist) { [/foo/, "bar"] }
+
+            it "validates the host" do
+              expect(PactBroker::Webhooks::CheckHostWhitelist).to have_received(:call).with("some.url", webhook_host_whitelist)
+            end
+
+            context "when the host does not match the whitelist" do
+              let(:whitelist_matches) { [] }
+
+              it "contains an error", pending: "need to work out how to do dynamic messages" do
+                expect(subject.errors[:"request.url"]).to include "host must be in the whitelist /foo/, \"bar\" . See /doc/webhooks#whitelist for more information."
+              end
+            end
+          end
+
           context "with no URL" do
             let(:json) do
               valid_webhook_with do |hash|

spec/lib/pact_broker/webhooks/check_host_whitelist_spec.rb

@@ -0,0 +1,47 @@
+require 'pact_broker/webhooks/check_host_whitelist'
+
+module PactBroker
+  module Webhooks
+    describe CheckHostWhitelist do
+      context "when the host is 10.0.0.7" do
+        let(:host) { "10.0.1.0" }
+
+        it "matches 10.0.0.0/8" do
+          expect(CheckHostWhitelist.call(host, ["10.0.0.0/8"])).to eq ["10.0.0.0/8"]
+        end
+
+        it "matches 10.0.1.0" do
+          expect(CheckHostWhitelist.call(host, [host])).to eq [host]
+        end
+
+        it "does not match 10.0.0.2" do
+          expect(CheckHostWhitelist.call(host, ["10.0.0.2"])).to eq []
+        end
+
+        it "does not match 10.0.0.0/28" do
+          expect(CheckHostWhitelist.call(host, ["10.0.0.0/28"])).to eq []
+        end
+      end
+
+      context "when the host is localhost" do
+        let(:host) { "localhost" }
+
+        it "matches localhost" do
+          expect(CheckHostWhitelist.call(host, [host])).to eq [host]
+        end
+
+        it "matches /local.*/" do
+          expect(CheckHostWhitelist.call(host, [/local*/])).to eq [/local*/]
+        end
+
+        it "does not match foo" do
+          expect(CheckHostWhitelist.call(host, ["foo"])).to eq []
+        end
+
+        it "does not match /foo.*/" do
+          expect(CheckHostWhitelist.call(host, [/foo*/])).to eq []
+        end
+      end
+    end
+  end
+end