working github webhook auth

Author: GrantBirki

lib/hooks/app/api.rb

@@ -57,9 +57,9 @@ def enforce_request_limits(config)
             end
 
             # Verify the incoming request
-            def request_validator(payload, headers, endpoint_config)
+            def validate_request(payload, headers, endpoint_config)
               request_validator_config = endpoint_config[:request_validator]
-              validator_type = request_validator_config[:type] || "default"
+              validator_type = request_validator_config[:type]
               secret_env_key = request_validator_config[:secret_env_key]
 
               return unless secret_env_key
@@ -115,8 +115,7 @@ def load_handler(handler_class_name, handler_dir)
                 require file_path
                 Object.const_get(handler_class_name).new
               else
-                # Create a default handler for POC
-                DefaultHandler.new
+                raise LoadError, "Handler #{handler_class_name} not found at #{file_path}"
               end
             rescue => e
               error!("failed to load handler #{handler_class_name}: #{e.message}", 500)
@@ -193,6 +192,9 @@ def determine_error_code(exception)
                   raw_body = request.body.read
 
                   # Verify/validate request if configured
+                  logger.info "validating request (id: #{request_id}, handler: #{handler_class_name})"
+                  logger.debug "raw body: #{raw_body.inspect}"
+                  logger.debug "headers: #{headers.inspect}"
                   validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator]
 
                   # Parse payload

lib/hooks/plugins/request_validator/github_webhooks.rb

@@ -26,6 +26,9 @@ def self.valid?(payload:, headers:, secret:, config:)
           signature_header = config.dig(:request_validator, :header) || "X-Hub-Signature-256"
           algorithm = config.dig(:request_validator, :algorithm) || "sha256"
 
+          signature_header = Utils::Normalize.header(signature_header)
+          headers = Utils::Normalize.headers(headers)
+
           provided_signature = headers[signature_header]
           return false if provided_signature.nil? || provided_signature.empty?
 

lib/hooks/plugins/utils/normalize.rb

@@ -0,0 +1,85 @@
+# frozen_string_literal: true
+
+module Hooks
+  module Plugins
+    module Utils
+      # Utility class for normalizing HTTP headers
+      #
+      # Provides a robust method to consistently format HTTP headers
+      # across the application, handling various edge cases and formats.
+      class Normalize
+        # Normalize a hash of HTTP headers
+        #
+        # @param headers [Hash, #each] Headers hash or hash-like object
+        # @return [Hash] Normalized headers hash with downcased keys and trimmed values
+        #
+        # @example Hash of headers normalization
+        #   headers = { "Content-Type" => "  application/json  ", "X-GitHub-Event" => "push" }
+        #   normalized = Normalize.headers(headers)
+        #   # => { "content-type" => "application/json", "x-github-event" => "push" }
+        #
+        # @example Handle various input types
+        #   Normalize.headers(nil)                    # => nil
+        #   Normalize.headers({})                     # => {}
+        #   Normalize.headers({ "KEY" => ["a", "b"] }) # => { "key" => "a" }
+        #   Normalize.headers({ "Key" => 123 })       # => { "key" => "123" }
+        def self.headers(headers)
+          # Handle nil input
+          return nil if headers.nil?
+
+          # Fast path for non-enumerable inputs (numbers, etc.)
+          return {} unless headers.respond_to?(:each)
+
+          normalized = {}
+
+          headers.each do |key, value|
+            # Skip nil keys or values entirely
+            next if key.nil? || value.nil?
+
+            # Convert key to string, downcase, and strip in one operation
+            normalized_key = key.to_s.downcase.strip
+            next if normalized_key.empty?
+
+            # Handle different value types efficiently
+            normalized_value = case value
+                               when String
+                                 value.strip
+                               when Array
+                                 # Take first non-empty element for multi-value headers
+                                 first_valid = value.find { |v| v && !v.to_s.strip.empty? }
+                                 first_valid ? first_valid.to_s.strip : nil
+                               else
+                                 value.to_s.strip
+                               end
+
+            # Only add if we have a non-empty value
+            normalized[normalized_key] = normalized_value if normalized_value && !normalized_value.empty?
+          end
+
+          normalized
+        end
+
+        # Normalize a single HTTP header name
+        #
+        # @param header [String] Header name to normalize
+        # @return [String, nil] Normalized header name (downcased and trimmed), or nil if input is nil
+        #
+        # @example Single header normalization
+        #   Normalize.header("  Content-Type  ")  # => "content-type"
+        #   Normalize.header("X-GitHub-Event")    # => "x-github-event"
+        #   Normalize.header("")                  # => ""
+        #   Normalize.header(nil)                 # => nil
+        #
+        # @raise [ArgumentError] If input is not a String or nil
+        def self.header(header)
+          return nil if header.nil?
+          if header.is_a?(String)
+            header.downcase.strip
+          else
+            raise ArgumentError, "Expected a String for header normalization"
+          end
+        end
+      end
+    end
+  end
+end

script/server

@@ -1,16 +1,10 @@
 #! /usr/bin/env bash
 
-# usage: script/server [--dev]
+# usage: script/server
 
 set -e
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd .. && pwd )"
 cd "$DIR"
 
-# check for --dev flag
-if [[ "$1" == "--dev" ]]; then
-  source script/dev
-  shift # remove the --dev flag from the arguments
-fi
-
 bundle exec puma -C spec/acceptance/config/puma.rb --tag hooks

spec/acceptance/acceptance_tests.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative "../spec_helper"
+
 require "rspec"
 require "net/http"
 require "json"
@@ -69,5 +71,36 @@
         expect(body).to have_key("timestamp")
       end
     end
+
+    describe "github" do
+      it "receives a POST request but contains an invalid HMAC signature" do
+        payload = { action: "push", repository: { name: "test-repo" } }
+        headers = { "Content-Type" => "application/json", "X-Hub-Signature-256" => "sha256=invalidsignature" }
+        response = http.post("/webhooks/github", payload.to_json, headers)
+
+        expect(response).to be_a(Net::HTTPUnauthorized)
+        expect(response.body).to include("request validation failed")
+      end
+
+      it "receives a POST request but there is no HMAC related header" do
+        payload = { action: "push", repository: { name: "test-repo" } }
+        headers = { "Content-Type" => "application/json" }
+        response = http.post("/webhooks/github", payload.to_json, headers)
+        expect(response).to be_a(Net::HTTPUnauthorized)
+        expect(response.body).to include("request validation failed")
+      end
+
+      it "successfully processes a valid POST request with HMAC signature" do
+        payload = { action: "push", repository: { name: "test-repo" } }
+        headers = {
+          "Content-Type" => "application/json",
+          "X-Hub-Signature-256" => "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_HMAC_SECRET, payload.to_json)
+        }
+        response = http.post("/webhooks/github", payload.to_json, headers)
+        expect(response).to be_a(Net::HTTPSuccess)
+        body = JSON.parse(response.body)
+        expect(body["status"]).to eq("success")
+      end
+    end
   end
 end

spec/acceptance/config/endpoints/github.yaml

@@ -1,6 +1,6 @@
 # Sample endpoint configuration for GitHub webhooks
 path: /github
-handler: GitHubHandler
+handler: GithubHandler
 
 # GitHub uses HMAC SHA256 signature validation
 request_validator:

spec/acceptance/config/hooks.yaml

@@ -1,6 +1,6 @@
 # Sample configuration for Hooks webhook server
 handler_dir: ./spec/acceptance/handlers
-log_level: info
+log_level: debug
 
 # Request handling
 request_limit: 1048576    # 1MB max body size

spec/acceptance/docker-compose.yml

@@ -8,6 +8,7 @@ services:
       - "8080:8080"
     environment:
       LOG_LEVEL: DEBUG
+      GITHUB_WEBHOOK_SECRET: "octoawesome-secret"
     command: ["script/server"]
     healthcheck:
       test: ["CMD", "curl", "-f", "http://0.0.0.0:8080/health"]

spec/acceptance/handlers/github_handler.rb

@@ -1,160 +1,16 @@
 # frozen_string_literal: true
 
 # Example handler for GitHub webhooks
-class GitHubHandler < Hooks::Handlers::Base
+class GithubHandler < Hooks::Handlers::Base
   # Process GitHub webhook
   #
   # @param payload [Hash, String] GitHub webhook payload
   # @param headers [Hash<String, String>] HTTP headers
   # @param config [Hash] Endpoint configuration
   # @return [Hash] Response data
   def call(payload:, headers:, config:)
-    # GitHub sends event type in header
-    event_type = headers["X-GitHub-Event"] || "unknown"
-
-    puts "GitHubHandler: Received #{event_type} event"
-
-    return handle_raw_payload(payload, config) unless payload.is_a?(Hash)
-
-    case event_type
-    when "push"
-      handle_push_event(payload, config)
-    when "pull_request"
-      handle_pull_request_event(payload, config)
-    when "issues"
-      handle_issues_event(payload, config)
-    when "ping"
-      handle_ping_event(payload, config)
-    else
-      handle_unknown_event(payload, event_type, config)
-    end
-  end
-
-  private
-
-  # Handle raw string payload
-  #
-  # @param payload [String] Raw payload
-  # @param config [Hash] Configuration
-  # @return [Hash] Response
-  def handle_raw_payload(payload, config)
-    {
-      status: "raw_payload_processed",
-      handler: "GitHubHandler",
-      payload_size: payload.length,
-      repository: config.dig(:opts, :repository),
-      timestamp: Time.now.iso8601
-    }
-  end
-
-  # Handle push events
-  #
-  # @param payload [Hash] Push event payload
-  # @param config [Hash] Configuration
-  # @return [Hash] Response
-  def handle_push_event(payload, config)
-    ref = payload["ref"]
-    branch = ref&.split("/")&.last
-    commits_count = payload.dig("commits")&.length || 0
-
-    # Check if branch is in filter
-    branch_filter = config.dig(:opts, :branch_filter)
-    if branch_filter && !branch_filter.include?(branch)
-      return {
-        status: "ignored",
-        handler: "GitHubHandler",
-        reason: "branch_not_in_filter",
-        branch: branch,
-        filter: branch_filter,
-        timestamp: Time.now.iso8601
-      }
-    end
-
-    {
-      status: "push_processed",
-      handler: "GitHubHandler",
-      repository: payload.dig("repository", "full_name"),
-      branch: branch,
-      commits_count: commits_count,
-      pusher: payload.dig("pusher", "name"),
-      timestamp: Time.now.iso8601
-    }
-  end
-
-  # Handle pull request events
-  #
-  # @param payload [Hash] Pull request event payload
-  # @param config [Hash] Configuration
-  # @return [Hash] Response
-  def handle_pull_request_event(payload, config)
-    action = payload["action"]
-    pr_number = payload.dig("pull_request", "number")
-    pr_title = payload.dig("pull_request", "title")
-
-    {
-      status: "pull_request_processed",
-      handler: "GitHubHandler",
-      action: action,
-      repository: payload.dig("repository", "full_name"),
-      pr_number: pr_number,
-      pr_title: pr_title,
-      author: payload.dig("pull_request", "user", "login"),
-      timestamp: Time.now.iso8601
-    }
-  end
-
-  # Handle issues events
-  #
-  # @param payload [Hash] Issues event payload
-  # @param config [Hash] Configuration
-  # @return [Hash] Response
-  def handle_issues_event(payload, config)
-    action = payload["action"]
-    issue_number = payload.dig("issue", "number")
-    issue_title = payload.dig("issue", "title")
-
-    {
-      status: "issue_processed",
-      handler: "GitHubHandler",
-      action: action,
-      repository: payload.dig("repository", "full_name"),
-      issue_number: issue_number,
-      issue_title: issue_title,
-      author: payload.dig("issue", "user", "login"),
-      timestamp: Time.now.iso8601
-    }
-  end
-
-  # Handle ping events (webhook test)
-  #
-  # @param payload [Hash] Ping event payload
-  # @param config [Hash] Configuration
-  # @return [Hash] Response
-  def handle_ping_event(payload, config)
-    {
-      status: "ping_acknowledged",
-      handler: "GitHubHandler",
-      repository: payload.dig("repository", "full_name"),
-      hook_id: payload.dig("hook", "id"),
-      zen: payload["zen"],
-      timestamp: Time.now.iso8601
-    }
-  end
-
-  # Handle unknown events
-  #
-  # @param payload [Hash] Event payload
-  # @param event_type [String] Event type
-  # @param config [Hash] Configuration
-  # @return [Hash] Response
-  def handle_unknown_event(payload, event_type, config)
-    {
-      status: "unknown_event_processed",
-      handler: "GitHubHandler",
-      event_type: event_type,
-      repository: payload.dig("repository", "full_name"),
-      supported_events: config.dig(:opts, :events),
-      timestamp: Time.now.iso8601
+    return {
+      status: "success"
     }
   end
 end